hasura / ra-data-hasura

react-admin data provider for Hasura GraphQL Engine
MIT License
336 stars 70 forks source link

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

Closed quentin-decre closed 1 month ago

quentin-decre commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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