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

build(deps): support react-admin v5 #170

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Conversation

quentin-decre
Copy link
Contributor

I updated package.json to support react-admin V5.

Ran tests successfully
Upgraded my large project (jarvi.tech), and it worked very well.

@quentin-decre
Copy link
Contributor Author

@praveenweb how about this small PR ?
Also, if you want, I'm ok to help contribute this package we use a lot for our project

@praveenweb
Copy link
Member

@quentin-decre - Thanks for the PR. Will test this out. But quick question: Is the intention of this PR to have backward compatibility with v4 as well? I haven't checked breaking changes.

Separately, I'm happy to support external contributions to the project in any way :)

@ValentinH
Copy link

@quentin-decre - Thanks for the PR. Will test this out. But quick question: Is the intention of this PR to have backward compatibility with v4 as well? I haven't checked breaking changes.

This part of the changes might be a breaking change though: https://marmelab.com/react-admin/doc/5.0/Upgrade.html#ra-data-graphql-and-ra-data-graphql-simple-no-longer-return-a-promise

@quentin-decre
Copy link
Contributor Author

No breaking changes for ra-data-hasura. It just works with the new version of react-admin so we just allow it to be installed with react-admin 5 as well as 4. The code has not changed.

@quentin-decre
Copy link
Contributor Author

quentin-decre commented Jul 12, 2024

@quentin-decre - Thanks for the PR. Will test this out. But quick question: Is the intention of this PR to have backward compatibility with v4 as well? I haven't checked breaking changes.

This part of the changes might be a breaking change though: https://marmelab.com/react-admin/doc/5.0/Upgrade.html#ra-data-graphql-and-ra-data-graphql-simple-no-longer-return-a-promise

ra-data-hasura does not return a promise for buildCustomDataProvider (the default export), so this is already compatible. We just do not need in our custom code to wrap this in a promise. So that's more straigthforward.

This PR is deployed in production, 200 000 graphql requests per day :)

@ValentinH
Copy link

@quentin-decre - Thanks for the PR. Will test this out. But quick question: Is the intention of this PR to have backward compatibility with v4 as well? I haven't checked breaking changes.

This part of the changes might be a breaking change though: https://marmelab.com/react-admin/doc/5.0/Upgrade.html#ra-data-graphql-and-ra-data-graphql-simple-no-longer-return-a-promise

ra-data-hasura does not return a promise for buildCustomDataProvider (the default export), so this is already compatible. We just do not need in our custom code to wrap this in a promise. So that's more straigthforward.

This PR is deployed in production, 200 000 graphql requests per day :)

Good point! 🙂

Impressive traffic you got here for an admin tool!🔥

@praveenweb praveenweb merged commit 6c97e71 into hasura:master Jul 19, 2024
1 check passed
@praveenweb
Copy link
Member

@quentin-decre - Thanks again for the confirmation. I just merged the PR and released a v0.7.0 for react-admin v5 support.

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.

3 participants