Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication #45

Merged
merged 8 commits into from
Aug 25, 2019
Merged

Authentication #45

merged 8 commits into from
Aug 25, 2019

Conversation

BGordts
Copy link
Collaborator

@BGordts BGordts commented Aug 25, 2019

  • Log in works
  • Registration works
  • Access tokens are now managed on the backend and not sent to the client anymore, which is more secure
  • Using firebase to manage login state

Note:

  • I'm not reusing one component between registration and login, even though these screens are very similar. The reason is that I'm not super sure whether they will stay that similar or not. Can still refactor when we make common changes.

@BGordts BGordts requested a review from ncarchedi August 25, 2019 04:15
@ncarchedi
Copy link
Owner

@BGordts I get this error in the console when I try to run this (even after running yarn):

The package at "node_modules/react-native-dotenv/index.js" attempted to import the Node standard library module "path". It failed because React Native does not include the Node standard library. Read more at https://docs.expo.io/versions/latest/introduction/faq/#can-i-use-nodejs-packages-with-expo
Failed building JavaScript bundle.

@BGordts
Copy link
Collaborator Author

BGordts commented Aug 25, 2019

I'm looking at this error now, that's really strange. I'll try to reproduce.

@BGordts
Copy link
Collaborator Author

BGordts commented Aug 25, 2019

Can you look at the solutions listed here? I think the 4th comment could work. zetachang/react-native-dotenv#39

Otherwise I'm happy to pairprogram on it. Problem is that I can't reproduce locally :(

@ncarchedi
Copy link
Owner

@BGordts ridiculous, but this was the thing that ended up fixing my problem: zetachang/react-native-dotenv#39 (comment)

Have to leave the house now for a bit, but will review/test/merge later today.

@ncarchedi ncarchedi merged commit 107e4e4 into master Aug 25, 2019
@ncarchedi ncarchedi deleted the authentication branch August 25, 2019 18:45
@ncarchedi
Copy link
Owner

Seems to work nicely, so I'll merge now to keep master up-to-date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants