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

react-admin 3.0 upgrade #67

Merged
merged 1 commit into from
Feb 21, 2020

Conversation

simonbuerger
Copy link
Contributor

@simonbuerger simonbuerger commented Dec 2, 2019

  • Removed RA realtime as per [RFR] Remove ra-realtime marmelab/react-admin#3908 (comment)
  • Implemented authProvider as object instead of function
  • authProvider.login method will now check for auth state change if no email and password is provided - enabling easy usage with third party Oauth firebase login flows (simply use useLogin without username and password OR specify '/' as the redirect URL as the checkAuth method will take care of making sure the user is logged in when redirecting)
  • Added example for usage with https://github.com/firebase/firebaseui-web-react resolves Full auth functionality #34
  • Make react, react-admin and react-dom peerDependencies
  • Update demo project
  • Update docs

@benwinding Should I include the dist files in the PR?

Pity about ra-realtime, but does not look like it is supported any longer. A solution to that seems beyond the scope of this upgrade and like a separate task?

Resolves #46 #34

@benwinding
Copy link
Owner

benwinding commented Dec 2, 2019

Wow amazing work @simonbuerger!

Thank you for your contribution! I agree that the ra-realtime issue is a separate task we can deal with afterwards.

Everything seems to work pretty well. And thanks for implementing the FirebaseUI in issue #34.

The only problem I found is that the login loader seems to be loading indefinitely as shown below, can you confirm this?

image

We'll need to merge it in before building the dist files and deploying too.

Kind regards,
Ben

@simonbuerger
Copy link
Contributor Author

@benwinding I'm actually not getting that - is it throwing any errors?

@simonbuerger
Copy link
Contributor Author

Oh and should I submit a PR to update the example repos? Or do you take care of those?

@LaszloDev
Copy link
Collaborator

@benwinding can you please merge this into the upgrade-to-react-admin-v3-branch so we can continue to work on it together? An beta NPM package for it would be great too.

BTW Merry christmas

@LaszloDev
Copy link
Collaborator

Will do the preparation to merge it with the v3 repo branch as soon as I’m back at my laptop.

@benwinding what is the general strategy regard publishing the NPM package, I do see that you checked in the /dist folder. Is that necessary and can we configure Github actions to automatically publish on master merge? With this we might need to introduce a dev or next-branch for the pull requests to be merged to, check them and then only merge to master.
I guess the versioning of the NPM package is done manually by you, before publishing?

@steurt
Copy link

steurt commented Dec 31, 2019

@benwinding can you please merge this into the upgrade-to-react-admin-v3-branch so we can continue to work on it together? An beta NPM package for it would be great too.

BTW Merry christmas

A beta NPM package would be great indeed!

Just started using react-admin (so v3) and want to use react-admin-firebase as well. Looks like the best Firebase provider for react-admin 👌🏻. However this v3 support is quite vital I think.

If a beta package would be released I could use it and test it. Looking to contribute to this already good and promising package 💪🏻.

@davidstackio
Copy link
Contributor

Also looking to use this package for react-admin v3. I agree this looks like the most promising package for using Firebase with react-admin. Happy to help test!

@LaszloDev LaszloDev mentioned this pull request Jan 5, 2020
7 tasks
@LaszloDev
Copy link
Collaborator

I added the needed /dist-files and pushed them to the upgrade-to-react-admin-v3-branch to have a working version. Please try it by linking directly to the github repo branch as package for now via:

npm install or yarn add github:benwinding/react-admin-firebase#upgrade-to-react-admin-v3

@benwinding has to publish a (beta) package to npm so it can be installed via npm

@LaszloDev
Copy link
Collaborator

@simonbuerger thanks again for your work.
I was not able to change the target branch for your pull request to the local upgrade-to-react-admin-v3, so I guess we have to create a new pull request so we can work together on finalizing it, as I would like to make some more changes, at least regards the readme before merging it to the master. Please check if you can change the target branch, otherwise I would close this pull request and open a new one.

@davidstackio
Copy link
Contributor

davidstackio commented Jan 6, 2020

@LaszloDev This is awesome, thanks! I was able to install and test with the command npm install --save github:benwinding/react-admin-firebase#upgrade-to-react-admin-v3

I tested reading and writing to a test collection in Firebase and it works just fine. Nice work @simonbuerger!

A note:
1. The authProvider didn't seem to be built into the dist so I couldn't test that with a collection only accessible by an authenticated user. Any way to update the dist so that can be tested?

EDIT: I figured out why the authProvider wasn't working for me. It was because I didn't use the loginPage={CustomLoginPage} as seen in the demo.

@simonbuerger
Copy link
Contributor Author

@LaszloDev I see you managed to merge this branch into upgrade-to-react-admin-v3, so you don't need me to update this PR?

@LaszloDev
Copy link
Collaborator

LaszloDev commented Jan 14, 2020 via email

@davidstackio
Copy link
Contributor

I figured out how to test the Google provider login and it works for me!

@benwinding benwinding merged commit 8b2d36a into benwinding:master Feb 21, 2020
@benwinding
Copy link
Owner

Hey Guys,

Sorry about the delay on this PR, seems to work pretty well so I've merged this in and deployed to npm under version 3.0.0 (starting at 3.0.0 to follow along with react-admin's major versioning)

Open a new issue if there's any concerns or problems. Thanks again for all your help and contributions, I really do appreciate it.

Kind regards,
Ben

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.

Update to React-Admin v3 Full auth functionality
5 participants