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-template: update to React 18 #52

Open
psimk opened this issue Apr 22, 2022 · 7 comments
Open

react-template: update to React 18 #52

psimk opened this issue Apr 22, 2022 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@psimk
Copy link

psimk commented Apr 22, 2022

React 18 has been released for around a month now (blog post), and its about time that we upgrade to it. The React team has promised that the upgrade should be fairly easy and smooth (outlined this blog post).

However, we should not just blindly bump the version numbers and follow the upgrade guide - we use a number of libraries and have utilities setup for them that might no longer be ideal. This requires some additional research and testing:

state management

For state management we are using mobx, and according to mobxjs/mobx#2526, it does not seem like mobx has or will have proper concurrent feature support. The issue boggles down to the maintainers suggesting to structure your app in such a way that the observable mobx store is separated from your React components, and all state is still managed by useState. In my eyes this makes things more complicated, and might only makes sense for large scale applications where we can control the architecture and be more strict with it. For most projects, we need something simple and easy to use.

I think we need to investigate other state management solutions, as a there is a lot to choose from now, and some of those solutions like zustand, do support concurrent features.

strict mode

StrictMode has become even more strict. I think we should disable it by default, but provide an easy way to enable it, so developers know that its is an option and something that they should test.

something else?

...

@psimk psimk added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 22, 2022
@ThaNarie
Copy link
Member

state management ... as a there is a lot to choose from now

That's opening a can of worms :D

Mostly if we should go (back) to something redux-like (what zustand is), and has both benefits and could be considered a bit more complex, versus something like mobx, which although potentially simpler, also has downsides.

https://recoiljs.org/ is also still a candidate, but not 1.0, so maybe not good enough to be using in production. It does support React 18 with concurrent though :) And seems to be somewhat in between the two above-mentioned options.

Do we consider this decision a blocker for updating to React 18, or would we consider upgrading but know that concurrent will not work nicely (or at all) until we switch state management later?

StrictMode has become even more strict. I think we should disable it by default

I'm wondering what the motivation behind that suggestion is?
Too noisy in the console? Too much work to address reported issues? Too much false positives?

@psimk
Copy link
Author

psimk commented Apr 25, 2022

StrictMode has become even more strict. I think we should disable it by default

I'm wondering what the motivation behind that suggestion is? Too noisy in the console? Too much work to address reported issues? Too much false positives?

React 18 has new features, but the developer does not have to learn and start using them right away, however because StrictMode got stricter, developers will have to adapt to its additional strictness.

The additional strictness boggles down to effects, and their cleanups running multiple times - this is for good reason, to force you to write your component state in a manner that is resilient between re-renders. However, if you are not used to writing your component state this way, then it might be a shock. As if for example you were doing an API call from your effect, and don't cache the promise (plain fetch) and/or don't use AbortController, you would fire of 2 requests.

So we can either:

A) disable it, avoid developer headaches and let them be free, but have less optimized apps by default.
B) enable it and force developers to learn how to write state that is reusable.

I am initially leaning towards A), as understanding what exactly StrictMode does is an important prerequisite for B) to work. As this is not as simple as following the "rules of hooks" or having an eslint rule that helps, you really do have to examine your effects thoroughly.

B) can be possible, if we make sure that the template covers:

  1. a lot of the more major and complex uses of useEffect, such as data fetching.
  2. add some base documentation next to the StrictMode wrapper, so developers know what it is doing and where to look for help in figuring out how to write reusable state.

@ThaNarie
Copy link
Member

Okay clear! I agree that we should go forward with A) now, and start working on B) to enable it down the road.

@crevulus
Copy link

crevulus commented Apr 26, 2022

Is there a resolution to the state management discussion yet or are we blocked until that is resolved?

My two cents: I'd vote for Redux with toolkit. I personally prefer zustand but you can't argue with Redux's prevalence and support. Using a widley-known library like redux means easier onboarding of new devs, and solid legacy support for platforms. As our React skeleton is largely used for platforms, we need to think what will the state management space be like in 5+ years. And in 5+ years I hope zustand will still be around, but I know Redux will be.

MobX is too OOP-y. The syntax is clunky and outdated, for me, and the fact they haven't updated for concurrent mode after it's been around for years is not a great sign.

@leroykorterink any comments on state manager perf? I see that Redux is much smaller than MobX, too 🙌

@leroykorterink
Copy link
Contributor

I think an external state management package could be a project specific problem to figure out. The React State and Context APIs are probably the best tools to use in a skeleton. Any state management package that we're going to add is an abstraction for those APIs anyway.

@psimk
Copy link
Author

psimk commented Apr 26, 2022

I think an external state management package could be a project specific problem to figure out.

I would agree with that, but only for the react-base template. The react template should be opinionated with the tools that we know work best for building great apps of any scale, this is why for example it contains storybook. Moreover, state management, depending on the library, can become quite boilerplate prone, so putting it on the developer to figure out, means that for each project, they might have redo (copy over) the same setup.

@leroykorterink
Copy link
Contributor

I don't think we need a state management library for every project, it can be something that a specific project needs (Can't really go into details here 😉) . I think the React State/Context APIs will be fine for 95% of the projects that we do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants