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

[RFC] React-admin v5 #9462

Open
15 of 18 tasks
fzaninotto opened this issue Nov 21, 2023 · 31 comments
Open
15 of 18 tasks

[RFC] React-admin v5 #9462

fzaninotto opened this issue Nov 21, 2023 · 31 comments
Labels
RFC Request For Comments

Comments

@fzaninotto
Copy link
Member

fzaninotto commented Nov 21, 2023

Rationale

It's been a year and a half since version 4.0. The React ecosystem has evolved a lot since then, and we must accept a few breaking changes in the react-admin codebase to keep up-to-date. So we're starting the work on the next major version, react-admin v5.

The main constraint of this version is to be a painless migration from v4. The 3.0 to 4.0 migration was costly for many developers, we don't want to reiterate that. The migration should take at most a day on a moderate size admin app.

Breaking Changes

See the progress in the following milestones:

Out Of Scope

  • Framework-agnostic library (to pave the way for Svelte Admin, Astro admin, etc)
  • Turn react-admin into a server-side rendering framework. We believe that SPAs are still the best architecture for admins.

Roadmap

We intend to release version 5 in early 2024.

There won't be any other major version until 2026, so we must include in v5 all the breaking changes that are necessary to last until then.

Version 5 is now the primary focus of the core team, so there won't be any new minor until we release v5. V4.16 is the last minor release in the 4.x branch. Every new feature will be added to the next branch, which will become react-admin v5.

https://github.com/github/release-radar

We need your feedback

Are there other breaking changes that you think should make it to v5? Please give us your feedback by commenting on this issue.

Note: Please don't comment with feature requests that don't imply any breaking change. For those, simply open a new issue.

@fzaninotto fzaninotto added the RFC Request For Comments label Nov 21, 2023
@fzaninotto fzaninotto pinned this issue Nov 21, 2023
@quentin-decre
Copy link
Contributor

I am aligned with the changes and what is out of scope. Useful parts for us :

  • Dom recycling for regular datagrid, or datagridAg with server loading
  • simpler RBAC (in the data provider), we will have to implement this in January
  • better support for Graphql (we use hasura), so that our data provider gets simpler.

Thanks for your very nice work !

@panfiva
Copy link

panfiva commented Nov 29, 2023

there are a few tickets that mentioned implementing them in v5. one of them is moving title to context. that would be a good change. #8872

@panfiva
Copy link

panfiva commented Dec 3, 2023

Change how setFilters work to make it more convenient to use:

  • Split setFilters into setFilters and setDisplayedFilters - most of the time, I either need to update filters or update displayed filters, but not both
  • Each function should accept the value itself, or a function (current_values)=>current_values (similar to setState):
    • Besides improving usability, also will allow running setFilter statements without worry of using outdated filters inside useEffect.
    • This is similar to setState when we want to increment current value by one twice: Instead of running setState(n+1); setState(n+1), we need to run setState((v)=v+1); setState((v)=v+1); I know that this example is over-simplified and no-one would write something like that; however I've been throw situations where calling setFilters (and setState) suffered concurrency issues.
  • Create setFilter(filter_name, value | (value)=>newValue ) and clearFilter(filter_name) functions to control individual filters.
  • Preserve referential integrity when filters do not change, and change references to the object when filters change (found the following comment in FilterForm.tsx: The reference to the filterValues object is not updated when it changes, so we must stringify it to compare it by value and also compare the reference.)

@Nilegfx
Copy link

Nilegfx commented Dec 7, 2023

  • I pretty much like the react-query upgrade
  • I would suggest to improve DX by introducing codemod for the breaking changes
  • I can't agree more with @panfiva's suggestions
  • could you please elaborate more on this one Remove cloneElement in inputs by introducing an input name prefix context. maybe an example?
  • Make all input width 100% by default (to make this page nicer) do you really want to have this huge stylistic breaking change as an implementation rather than documenting how to change theme/use grids to achieve it? i feel this will be the most annoying change for us (developers).

@KilianB
Copy link

KilianB commented Dec 13, 2023

  • could you please elaborate more on this one Remove cloneElement in inputs by introducing an input name prefix context. maybe an example?

As far as I can remember cloneElement is used to augment the path of nested form controls e.g. inside ReferenceManyInput to have a unique location for the purpose of react-hook-forms useController . One thing that cost me hours was the inability to nest layout elements inside certain components as they would expect the direct children to be the input/field components. This would be fixed with this change right?

Does the rework of the stricter Typescript types include generics for input components? Those would save a lot of issues and if registered Resources could be types, this help a lot during development

@panfiva
Copy link

panfiva commented Dec 14, 2023

Evaluate migration to future.v7_relativeSplatPath flag that react-router team implemented to address breaking change that they made in 6.19.0.

https://github.com/remix-run/react-router/blob/main/packages/react-router-dom/CHANGELOG.md#6210

@djhi
Copy link
Contributor

djhi commented Dec 15, 2023

As far as I can remember cloneElement is used to augment the path of nested form controls e.g. inside ReferenceManyInput to have a unique location for the purpose of react-hook-forms useController . One thing that cost me hours was the inability to nest layout elements inside certain components as they would expect the direct children to be the input/field components. This would be fixed with this change right?

Yes, that's the plan. Some high level components might still inspect children though, Datagrid for instance.

Does the rework of the stricter Typescript types include generics for input components? Those would save a lot of issues and if registered Resources could be types, this help a lot during development

Yes, we have plans for this. Not necessarily in v5.0 though

@milutinovici
Copy link

Consider switching to
https://github.com/TanStack/router?

@fzaninotto
Copy link
Member Author

Consider switching to https://github.com/TanStack/router?

Why would we want to do that? What problems does react-router have that tanstack router would solve?

@milutinovici
Copy link

This blog post explains all the advantages

https://swizec.com/blog/tanstack-router-modern-react-for-the-rest-of-us/

@fzaninotto
Copy link
Member Author

@milutinovici I don't understand the pain points that TanStack router would solve in react-admin apps.

@DiegoAndai
Copy link

Hey! Material UI has announced its v6 release for 2024 focusing on a new style engine: mui/material-ui#38137

The timeline (kind of) aligns with React-admin's v5, and I thought you might be interested, especially given there won't be any other major version until 2026.

Let us know! 😊

@fzaninotto
Copy link
Member Author

They announce v6 for Q2 2024... Which is late for us. It could mean up to 7 months without a new minor release for react-admin, which is too much. Besides, we'll need to wait until the dust settles e bit before using it, so I don't see our development on the new MUI starting before fall 24.

IMO, their schedule isn't compatible with ours, and react-admin v5 won't use MUI v6.

@davidhenley
Copy link
Contributor

Make all input width 100% by default (to make this page nicer)

Finally!

@smeng9
Copy link
Contributor

smeng9 commented Feb 29, 2024

I suggest we also update the documentation website to show the documentation for a specific branch (e.g. next as unreleased) in the dropdown selection before v5 is officially released. After v5 release we can then decide whether to remove that item from the dropdown list.

I propose we can do something like ESLint documentation in eslint/eslint#18139
Screenshot 2024-02-29 at 10 00 10 AM

This feature request may be a breaking change for the current ci pipeline, but it will be particularly helpful for early adopters to try and give feedbacks without the need to build and serve the documentation locally. During the upgrade cycle from v3 to v4 we don't have such thing for a better developer experience and a smoother migration.

@smeng9
Copy link
Contributor

smeng9 commented Mar 20, 2024

Hi @fzaninotto any plans on updating the docs on the https://marmelab.com/react-admin/documentation.html for v5 alpha?

@fzaninotto
Copy link
Member Author

@smeng9 No, we don't have such plans. The docs on the react-admin website will only be updated when we release v5 stable. In the meantime, you can browse the V5 docs at https://github.com/marmelab/react-admin/tree/next/docs.

@atilbian atilbian unpinned this issue Mar 29, 2024
@fzaninotto fzaninotto pinned this issue Apr 8, 2024
@MohammedFaragallah
Copy link
Contributor

Hello thank you for all your hard and amazing work

I see that you're planning on updating dependencies to their last major does that include date-fns? I can work on a PR.

@fzaninotto
Copy link
Member Author

@MohammedFaragallah yes, good idea! PR welcome.

@lefuturiste
Copy link

On the issue of dynamic title #5893
If I understand correctly, the goal is to automatically update the document title from the title in TitlePortal.
The PR that implemented this behaviour #6119 was reverted in #6357 because of an hidden breaking changes.
So may be the v5 is the right time to re-introduce this patch/PR?

@MohammedFaragallah
Copy link
Contributor

MohammedFaragallah commented May 1, 2024

@fzaninotto what do you think about updating these packages

  • react-dropzone v12 to v14
  • clsx v1 to v2
  • query-string v7 to v9
  • react-i18next v13 to v14
  • react-error-boundary v3 to v4

@fzaninotto
Copy link
Member Author

@MohammedFaragallah I'm all for it, please feel free to open one PR per dependency

@fzaninotto
Copy link
Member Author

On the issue of dynamic title #5893 If I understand correctly, the goal is to automatically update the document title from the title in TitlePortal. The PR that implemented this behaviour #6119 was reverted in #6357 because of an hidden breaking changes. So may be the v5 is the right time to re-introduce this patch/PR?

Yes, it's a good time. Would you like to try and solve the issue?

@tauqeer-trailfive
Copy link

hi @fzaninotto will you guys gona bumped up react-admin with React v19 as it almost around the corner?

@fzaninotto
Copy link
Member Author

@tauqeer-trailfive Do you know when it is supposed to be released?

@tauqeer-trailfive
Copy link

No it don't know but its beta release week ago.

@tauqeer-trailfive
Copy link

just wondering, we have to upgrade our code bases also if you're doing this.

@fzaninotto
Copy link
Member Author

We hope to release react-admin v5 not too far in the future (ideally in a month from now). I doubt React 19 will be up at that time.

Also, which react 19 features do you think would benefit to react-admin ?

@MohammedFaragallah
Copy link
Contributor

what do you think about adding generics to infer the returned props types from sanitize rest functions?

something like this

const sanitizeRestProps = <T extends Record<string, unknown>>({
    record,
    resource,
    initialValues,
    translate,
    ...rest
}: T) => rest;

rest here is typed Omit<T, "record" | "resource" | "initialValues" | "translate"> which is far more useful than the previous any

I can work on a PR

@fzaninotto
Copy link
Member Author

fzaninotto commented May 8, 2024

what do you think about adding generics to infer the returned props types from sanitize rest functions?

I don't see the interest for the end user, and I don't see it as a breaking change. We do a v5 especially for breaking changes. So while your suggestion might bring some improvements to the core team, it won't be a priority.

@MohammedFaragallah
Copy link
Contributor

I don't see the interest for the end user, and I don't see it as a breaking change.

I disagree i just noticed this while writing a custom input and I thought my experience would have been better if I had the returned props types and also if the sanitizers were exported instead of having to copy-paste them and keep them in sync

I understand if this is not a priority RN though 🤝

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

No branches or pull requests