-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Get rid of Immutable.js #3853
Comments
Huge thumbs up! This is a tech debt, but will require a breaking change so probably for v3 |
We already have |
Hey, I want to start working on this. Could you guide me a bit? What would be the steps if I, for example, want to remove Immutable from https://github.com/netlify/netlify-cms/blob/master/packages/netlify-cms-core/src/reducers/auth.js? |
Thanks @smashercosmo! I would first search where the For example here is one place: Then replace those direct access to the const selectUser = (auth) => auth && auth.get('user')
export { selectUser } We keep selectors in the same file as the relevant reducer, for example: Once everything is behind selectors, you would mostly need to change the selectors internal implementation and it will be easier to get rid of Immutable. Does that make sense? |
Yes. It does. Thank you) |
@erezrokah ok, I have a PR ready with immutable removed from 'status' and 'auth' state slices. What's the commit type for this kind of change? |
@smashercosmo that's great. I think you can use |
@martinjagodic is this still on the roadmap? Would you accept PRs which refactor immutable.js gradually out? (For all of the reasons stated in this issue, plus 3.5 years of tech debt interest) |
@mmkal that would be amazing, we would be very happy to accept this. However, this is a huge undertaking, so I guess bite-sized PRs would be a way to go. If you want, we can discuss this over Discord in more detail. |
Is your feature request related to a problem? Please describe.
Describe the solution you'd like
Standard solution to deal with immutable data structures nowadays is Immer.js
https://github.com/immerjs/immer
Additional context
I know it will be a lot of work, because Immuatble.js is used across the whole codebase. And I really don't know how to introduce it gradually. But I'd really like to help.
The text was updated successfully, but these errors were encountered: