In StrictMode, the useState() initializer function is called twice, but one of the results is discarded

React version: 17.0.1
React reconciler: 0.26.0

Steps To Reproduce

Link to code example: https://codesandbox.io/s/r3f-contact-shadow-forked-iggyv?file=/src/index.js:308-745

let log = console.log

let id = 0
function Obj(node) {
  this.id = id++
  log('constructor', this.id)
  node.addEventListener('wheel', () => log(this.id, 'wheel'), false)
}

function App(props) {
  // The object is created twice, why does react do that, it's not documented
  const [obj] = useState(() => log('new object') || new Obj(document.body))
  return <div>{obj.id}</div>
}

ReactDOM.unstable_createRoot(document.getElementById('root')).render(<App />)

old demo

https://codesandbox.io/s/r3f-contact-shadow-forked-e44m3?file=/src/index.js

This demo creates a local object which is supposed to live within the components lifecycle.
For some reason concurrent mode creates two versions of that object, but one is stuck in the view section.
These controls aren't allowed to zoom, yet, when you give you mousewheel - it zooms. The control clearly receives the flag.

This does not happen in blocking mode and previous reconcilers (for instance react 16.4.x and reconcilers pre 0.26

Debugging in this is almost impossible as React swallows console.logs. Some users have found out that it indeed creates two branches of local state: https://twitter.com/belinburgh/status/1319990608010874883

The state object (orbit-controls) has side-effects, it creates events, but that is and should be of no consequence.

Debugging in this is almost impossible as React swallows console.logs.

We're planning to make this configurable in DevTools (#19710). I agree this is pretty surprising, and unfortunately it's surprising the other way around too. For common application debugging we've found that double-logging (from double renders) is more annoying than silencing the extra render passes. But I certainly understand that the first time you discover this behavior, it's perplexing.

As a temporary workaround you can just do let log = console.log at the top level and then it'll always log.

For some reason concurrent mode creates two versions of that object,

I don't quite understand why you find this surprising. This is (and has always been) the behavior in StrictMode, too. (E.g. here's ReactDOM 16 with StrictMode running the state initializer twice.)

State initializer is part of render, and should be pure. Running it twice (and ignoring the result of one of the runs) should be completely safe, since the state initializer shouldn't mutate anything or add any listeners.

Why is running it twice a problem for you? It feels like the rest of the description assumes deep knowledge of the code so it's hard to comment on, but I can only speak to the expected behavior. Maybe you can reduce the code to a specific pattern that's breaking for you?

Here is a version that logs everything (by doing the let log = ... trick): https://codesandbox.io/s/r3f-contact-shadow-forked-229sw?file=/src/OrbitControls.js

The thing that immediately jumps out to me is the addEventListener inside the OrbitControls constructor. This is similar to if you called addEventListener during render. It's a side effect, which should not happen during the render phase. The fact that OrbitControls has a dispose method is a further confirmation of guess — if something needs to be "cleaned up", it clearly performs a side effect!

I think what you'd usually want to do is to split apart object creation and any side effects. E.g. new OrbitControls() should just initialize the fields, but there could be an attach() method that adds event listeners, to mirror the dispose() method. Then you call attach() from a layout effect. Effects are mirrored — so there you actually have initialization + destruction as a pair.

orbitcontrols is not mine, it is a threejs primitive, i forked it for more clarity. everything has side-effects and that is something i expect react to handle simply because we don't get to decide if our dependencies are clean or not. if we pull something from npm, remark, threejs, or whatever, and we need to create a local instance (like the react docs show), it must work, or else you simply wouldn't be able to have local state in the real world. r3f for instance could close ship because all of threejs works like that.

i don't really care if it creates it twice, but it seems like it then mixes up the two that it has created. or at least we would need a way to at least dispose of the one that CM has created. this bug was introduced in react-reconciler 0.26.0. the one before is not affected in concurrent mode.

If it has side effects, it needs to be done during the commit phase (eg useLayoutEffect).

the problem is that in almost every case you create a local instance of something, you need it in the view. here you do it with Remark: https://reactjs.org

Whatever has changed since the previous reconciler, this pushes us into more boilerplate and indirection.

const controls = useRef()
useLayoutEffect(() => {
  controls.current = new Controls()  
}, [])
return <foo controls={controls.current} /> // doesn't exist, it would need an additional forceRefresh

or worse

const [controls, set] = useState()
useLayoutEffect(() => {
  set(new Controls())
}, [])
return controls
  ? <foo controls={controls.current} />
  : null

i don't really care if it creates it twice, but it seems like it then mixes up the two that it has created

I’m not really sure what “mixes up” means here. Reducing this example to something with no third-party code would help a lot because I don’t know much about what this third-party code is supposed to be doing and I’m not familiar with the terminology. If this is truly a React bug you should be able to reproduce it with a tiny example with ReactDOM.

it must work, or else you simply wouldn't be able to have local state in the real world.

I would really appreciate if we could focus on the concrete issue and not be alarmist here. Doing a side effect during a constructor (like subscription) has always been a problem — for example, it’s completely broken if you use server rendering. I understand that R3F doesn’t, but this is not a new constraint overall for idiomatic React code. I’m sure we can figure out a solution here but a smaller self-contained example would make this much easier.

before i attempt that, could you explain what it does?

i have a local state variable

const [controls] = useState(() => new OrbitControlsImpl(camera, gl.domElement))

why does concurrent mode create two controls? if i understood this a bit better i could try to make a reduced variant for react-dom. the debug log thing made it seem like magic, but now at least i know that there are to instances running. one has the zoom flag, the other --- for the strangest reason --- didn't get to the view part where enableZoom=false is set, and this i think is the bug in react.

here you do it with Remark: https://reactjs.org

I want to clarify that the problem is not in creating instances. The problem is if the constructor of that instance has some side effects (such as setting up a DOM listener). This problem is not new. It is in fact the reason componentDidMount (or useLayoutEffect) exists. Separating code with side effects is what lets React support server rendering and other features in the future.

i understand this part. but can you explain why one control has enableZoom=true and the other doesn't? that is the bug.

the view says: <object {...props} /> and props contain enableZoom=false.

even if CM creates two versions of orbitcontrols, one does not get to the view and the props, it is interrupted for some reason before that. that is what i need to know before i can try a react.dom version.

why does concurrent mode create two controls?

This is not specific to Concurrent Mode. Again, in Strict Mode we always run the render two times in development. The second render will not "reuse" the state initializer — both of them will create one. It's just that we'll throw away the result of one of these renders in DEV.

Why do we do this? Precisely to find side effects during initialization, since that breaks other features.

one does not get to the view and the props, it is interrupted for some reason before that

Concurrent Mode can't literally "interrupt" anything in your code. We're still living according to the rules of JS. If enableZoom is not what you expect, it means that something in that class's code is setting (or not setting) it. You can add a log to every place it's being set to diagnose it.

I'd like to better understand the desirable behavior.

These statements are unclear to me:

one is stuck in the view section.

What do you mean by "stuck"? What is the "view section"?

one has the zoom flag, the other --- for the strangest reason --- didn't get to the view part where enableZoom=false is set

Where is that being "set"?

It would help if you could write step by step what you expected, and what is the actual behavior.

I’m finding it difficult to continue this conversation because most of the content of my replies seems to be ignored by your replies, and we’re beginning to walk in circles. But I will try to retrace my train of thought one more time. I don’t intend to keep repeating the same things over and over though, especially not on a weekend. I trust that you mean well though.

this is not about SSR

Indeed, your particular use case is not about SSR. But the context in which React users tend to encounter this problem is usually related to SSR. I hope you can agree that React server rendering is used much wider than R3F. Therefore, it might make sense that even if we wanted to solve this somehow for R3F, a solution that would not also solve the same problem for SSR would not be sufficient. We can't raise a magic wand and make code like node.addEventListener work on the server. And the server needs to calculate the initial state. Therefore, it’s a part of the React component contract that side effects should not happen during the state initializer. And again, it is precisely why componentDidMount (or useLayoutEffect) exists.

You did not write Remarkable, if remarkable has side-effects, and MUST live in the component, then you need a solution.

If something has side effects during initialization, the idiomatic solution is to separate them (e.g. move them into a separate method). Even if you did not "write" some code, often you are able to send a pull request or refactor the imperative code. Of course, I understand you don't want to rewrite Three.js, but I also don't get the feeling that this pattern is pervasive there. From looking at a bunch of internals, so far I've only found it in the *Controls modules, of which there are fewer than ten. In fact, some of these "controls" already have connect() and disconnect() method pairs — probably because someone already ran into the same issue and added them. This seems like a reasonable feature request for Three.js from my perspective. I can file it on your behalf, if you would prefer so.

You're right not every problem can be fixed at the source. The canonical workaround then, is to move this work to the commit phase (componentDidMount in classes, useLayoutEffect in Hooks). This is exactly what people do when they have this problem, can't fix it at the source, and need to fix their component to work with server rendering. Again, I understand your concern is unrelated to server rendering. I am saying that if the community has managed to figure it out so far for seven years, I'm sure that either of the two solutions I just enumerated can be made to work with your case too.

3rd party objects have side-effects and that's just the way it is. React needs to be able to hold them.

You are continuing the pattern of saying things "just" "need" to be done in some particular way. This is not conductive to a good technical discussion and I am asking you to please not do that.

I do want to point out (again) that "objects" can't "have side effects". This doesn't have a technical meaning. You are referring to constructors with side effects. I explained how those are handled right above.

Class component constructor, render, and shouldComponentUpdate methods

This does not include useState, which was supposed to be guaranteed as a unique state holder.

I am not sure what you mean by this. The useState initializer has the same role as a constructor. I don't know what "unique state holder" means but I'm assuming you are referring to the double invoking in StrictMode. However, the constructor is also double-invoked in StrictMode. So they are equivalent in that sense. If you think this caveat is valid with a constructor, I don't know why you don't like that it applies to useState.

You are saying outright that holding 3rd party objects is not a supported pattern.

No, this is not what I was saying. I feel like we're going to have to stop this conversation if you continue to insist that I said something that I didn't. I hope that this comment (and the two approaches in the beginning of it) help clarify my point.

That would mean React is incompatible with vanilla js. Anything in vanilla that adds event handler in the constructor or writes to global space is "not supported" - please let that sink in.

Let me once again point out that this is how React has worked for seven years for server rendering. I understand that you might not have used that, but all third-party component authors for ReactDOM eventually learn about this problem, and solve it with one of the two ways I have described in the beginning of this problem. Given the size of the React ecosystem, I think it's fair to say that this problem is solveable and has not led to any projects "dying".

All i am asking is if React could not invalidate a local state object that i have explicitely set up, knowing about the implications of SSR and so forth.

React does not "invalidate" anything. If the constructor did not have a side effect, it would have been completely safe for React to recreate it. Let's focus on the root of the problem — the double rendering is only a mechanism to surface it. And as I have said repeatedly, in this comment and others, the way to solve it is either in the imperative code or by moving this code to the commit phase.

with this new policy above

This is not a "new policy". React has existed for seven years, and for those seven years people have put side effects into componentDidMount because otherwise it breaks server rendering. It's not something React can "fix" in principle.

Let's take your component here:

#20090 (comment)

Now let's run it in Node:

It errors, because we're unable to calculate the initial state (document doesn't exist in Node, and neither does node.addEventListener). We can't calculate the initial state, so we can't render the component to HTML. I hope you see that this is not an arbitrary limitation imposed by React, and it is not a new limitation either.

As for double-invoking of constructors, it has been on for ReactDOM users for two years in <StrictMode> (and on since the beginning for useState initializers). StrictMode itself has been on by default for several months in Create React App. I think that given the evidence I have provided, it is plain wrong to claim that there is some "new policy" here.

In fact, the very word "policy" is misleading. What you're describing ("just make this work for SSR") is plainly not possible because the DOM does not exist on the server. It's not our whim, but a consequence of React being able to render things on the server (which is a good feature!) Coincidentally, the libraries that don't care about server rendering (e.g. jQuery) are the ones that don't enforce this separation of side effects.

react-three-fiber is essentially dead.

I believe this comment to be hyperbolic based on the evidence I have provided above. Let me recap:

This separation has always existed in ReactDOM, and it has not impeded its adoption over seven years
I've only seen this pattern in ~10 files in Three.js so far, although I concede there may be more — but it may actually be easily fixable at the source with adding connect and disconnect methods to a few more files
It has a workaround (layout effects) even when you can't fix it at the source

We can keep brainstorming but like I already wrote, I'm going to need a concrete example of what doesn't work with my approach, so that I can iterate on it.

However, to continue this discussion, I have to ask you to avoid hyperbolic statements like "React must do X or else", "this will kill my project", etc. I'm fairly certain we can find a solution to this that both you and I would be happy with, but this isn't conductive to finding it. Instead, let's focus on the actual problem. Thank you.

@Aprillion

@Aprillion as i said, this just makes a mess, because something that is available immediately is now being spread over 2 render phases and everything else has to adapt. normally a local variable is being used by other variables, hooks and the view. this now has to go in states and every step in the way you check against undefined. and that is just not what you can expect people to know. they'll run into this for ever.

there has to be some sort of compromise. if something doesn't work in SSR in a non SSR app, it should cause such a problem, especially if it's not under your own control.

@gaearon

@gaearon i do understand the purpose of it. these apps do not run under SSR, threejs itself can't. all of three works that way, every object they have has handlers or effects in the constructor. that is why im getting anxiety. if there is nothing for these cases, no possibility to hold a fixed state obj in a non ssr app, that endangers the project. im seeing these elaborated useLayoutEffect things spanning multiple render phases and this is no solution. i was hoping for a compromise, something that helps our projects and the users that create imperative objects all day, because that's the way it works.

sometimes escape hatches like dangerouslySetInnerHTML are needed. imagine you'd take that out because it's dirty. it would collapse so many usecases.

import React, { ____useEffectfulState } from 'react'

function Foo() {
  const foo = ____useEffectfulState(() => new Bar(), [conditions])

all of three works that way, every object they have has handlers or effects in the constructor.

Can you point me to concrete examples, please?

im seeing these elaborated useLayoutEffect things spanning multiple render phases and this is no solution.

Don't forget that you can wrap Hooks into components and then the user doesn't need to write them over and over again.

const foo = ____useEffectfulState(() => new Bar(), [conditions])

The problem here is not a lack of "effectful state". The problem is a side effect in constructor. This breaks server rendering and this breaks Concurrent Mode. (Which is why Strict Mode always surfaced this problem.) If this is a blocker for R3F, it can keep using Legacy Mode. But I can't "change the math" and magically make this pattern work. It's the constraint itself (no side effects in constructor) that enables these two features (server rendering and CM). Without this constraint, neither of these two features can work. This is the point of React — the reason it's able to add features like this is because it adds some constraints, compared to something like jQuery or vanilla Three.js. It's okay if you don't use all React features.

To stop going around in circles, please let's get back to this:

I transport non-scene objects into reacts vdom so that it becomes accessible to react. For instance now i can animate these controls with react-spring.

This is getting more concrete, thank you. I would like to ask you to keep the topic on these concrete examples of what doesn’t work with my suggested approach because they’re very helpful. Unfortunately your link to the API hasn’t been enough for me to understand what this means. It would help a lot if you could provide a sandbox based on your original one that does something which my approach doesn’t allow. Then I can better understand the concrete issue.

This is the most actionable thing you can do to help me understand the issue better. This will be much more helpful than discussing a proposal to change React.

i mean, it's a react element now, you can do anything to it. animate it for instance. you can't animate a hook.

function Foo({ open })
  const [foo] = useState(() => new Foo())
  const props = useSpring({ open: open ? 0 : -100 })
  <a.primitive object={foo} {...props} /> // this will animate "foo.open" 60fps outside of react

but this is besides the point. it could be return null and still have the same problem.

Can you point me to concrete examples, please?

everything in there has side-effects: https://github.com/mrdoob/three.js/tree/dev/examples/jsm/controls many other places in threejs as well.

but this is something you'll find elsewhere, too. any generic vanilla js thing.

for instance i just googled "vanilla js gesture" and they all work like that, for instance: https://github.com/zingchart/zingtouch

// vanilla
let zt = new ZingTouch.Region(document.body) // adds events

// react, yields two conflicting gesture handlers
function Zing({ flag }) {
  const [zing] = useState(() => new ZingTouch.Region(document.body))
  useMemo(() =>zing.doSomething(flag), [zing, flag])
  return <div>{zing.whatever}</div>

so react says, not supported. but with useLayoutEffect it just turns into a soup.

function Zing({ flag }) {
  const [zing, set] = useState()
  useLayoutEffect(() => {
    set(new ZingTouch.Region(document.body))
  }, [])

  useMemo(() => {
    if (zing) {
      zing.doSomething(flag)
    }
  }, [zing, flag])

  return zing
    ? <div>{zing.whatever}</div>
    : null

i mean, it's a react element now, you can do anything to it. animate it for instance. you can't animate a hook.

You can, if it's using the React render cycle. If you're not for performance reasons (as is the case with react-spring?), then I imagine that Hook could be hooked up to whatever mechanism that react-spring is already using. What is the low-level primitive through which <animated.*> components are implemented? How are they notified to update themselves?

everything in there has side-effects: https://github.com/mrdoob/three.js/tree/dev/examples/jsm/controls

Yes, but you previously said "I can't ask threejs to write 10.000 objects from scratch". You are now linking to a directory that contains literally eight modules, out of which three have already implemented connect / disconnect-like mirroring that we need. This matches what I already said earlier (and that you have not responded to):

I've only seen this pattern in ~10 files in Three.js so far, although I concede there may be more — but it may actually be easily fixable at the source with adding connect and disconnect methods to a few more files

I'm curious if you see this pattern in any other places in Three.js, to back up the claim of needing to "write 10.000 objects from scratch". It seems like it's important to assess the extent of this so we can pick a good solution.

for instance i just googled "vanilla js gesture" and they all work like that, for instance:

I believe I've already replied to this argument and I have nothing to add there.

I would like to propose one last time that you provide a CodeSandbox that demonstrates something that works with your current approach but doesn't work with mine. As I understand, this react-spring example is what you were referring to. Any chance you could make a working sandbox with it? Again, I'm not familiar with Three.js, so it's hard for me to make one.

@mrdoob

i have explained that this has nothing to do with it. i can make an example and this would be completely off the point.

the point is, i can't force three. they won't change it. 10 or 10.000 doesn't matter. it would affect their entire userbase of vanilla users that may not know what SSR frameworks do. i mean let's ask @mrdoob here ← would you please rewrite all objects with constructor side-effects (all controls, some shader based primitives, some lights, rectarea for instance, anything that adds events or writes into the global namespaces)?

i can't also force "zing" to stop using events in the constructor. all im looking for is a solution for non SSR apps that doesn't force me to route something through render phases just to create an object.

@mrdoob

i have explained that this has nothing to do with anything.

I'm not sure what you mean by this. A concrete runnable example that doesn't work with my proposed solution but works with your initial one is the most direct way for us to get to solving this issue. Assuming you're interested in solving the problem, this is the most actionable thing you can help me with. I can try to make one myself but it will take me a lot more time, since I'm not sure what property I can animate in OrbitControls and how to do that with react-spring. Or what other class I should be using that demonstrates the same problem.

you know how react-spring works. or react-natives animated

You are vastly overestimating my knowledge. I have a faint idea of how it works, but I don't know where that happens in the code and how exactly it is hooked up. So a concrete example would help me figure this out a lot quicker.

the point is, i can't force three. they won't change it. 10 or 10.000 doesn't matter. it would affect their entire userbase and a project that runs without semver makes that worse for them.

We can definitely try to send a PR to them that adds two methods to five files. This is not breaking semver in any way. (The default behavior can stay "connected" — we just need the ability to disconnect and later connect again.) This is not the only solution for sure, but it's worth at least assessing the amount of work here. I can send this PR myself if you don't want to get involved in this.

let's ask @mrdoob here ← would you please rewrite all controls and all objects with constructor side-effects

This is not what I'm asking for. If the side effects can be undone (e.g. some of these controls already offer a disconnect method), this would already solve our problem in the near term. Also, it's unfortunate that you have mentioned the maintainer here, since they will now be spammed with all of our follow-up comments that are concerned with React. I don't think this is a great way to get their attention — unless your point was to predispose them against solving this.

I think I've said it many times but I'll repeat: if we can't modify the underlying code, the workaround is to do this in the commit phase. I have provided one solution for commit phase that works. If you have constraints that make it a non-starter (e.g. the animation case), I need to see the problem for myself in a sandbox. There isn't much I can help with until one is provided. Thank you.

@drcmda

I've had a brief chat with the Three.js maintainer who indicated that they also think the problem is only relevant to the Controls, and that they're open to changing it to a connect / dispose pattern so that the side effects are not applied in the constructor. Controls (like other files in the examples folder) are not bound by semver, and people have had to update them due to other reasons recently anyway. So this change should be doable.

If we make this change in Three.js, it means that you'd still need to do useLayoutEffect to attach and detach the handlers, e.g. something like

function Controls() {
  const { camera, gl } = useThree()

  // OK if we remove side effects from the constructor
  const [controls, setControls] = useState(() => new ObjectControls(camera, gl.domElement))

  useLayoutEffect(() => {
    controls.attach()
    return () => controls.detach()
  }, [controls])

  //  ...
  return <primitive object={controls} />
}

But you would be able to keep using <primitive> in this case, which I think would solve your animation concern.

You can't really avoid this layout effect because you need to clean up somehow when the component is unmounted anyway. I respect your position that code like this is a "soup" but this is how ReactDOM users bind to window event handlers, so it's hardly out of the ordinary, and can always be made nicer with a custom Hook.

// Application code

function Controls() {
  const controls = useControls((camera, node) => new ObjectControls(camera, node))
  return <primitive object={controls} />
}

// ...
// Inside R3F:
// ...

function useControls(create) {
  const { camera, gl } = useThree()
  const [controls, setControls] = useState(create(camera, gl.domElement)

  useLayoutEffect(() => {
    controls.attach()
    return () => controls.detach()
  }, [controls])

  return controls;
}

If this is still too verbose for your taste, we could conceivably even build this whole logic into the <primitive> itself by placing it into commitMount and removeChild, respectively — if you detect Three.js objects with an attach method (or whatever we call it). So it really depends on how implicit you want to make it, although I'd suggest an explicit approach.

@drcmda If we solve this on the Three.js level with an approach like this, would it be satisfactory to you?

@drcmda

Just to close the loop on this.

We've talked with @drcmda, and he applied my earlier proposed fix:

When constructor has no side effects, simply useState
When constructor has side effects, move initialization to useLayoutEffect

The fix is in pmndrs/drei@47fd6a4.

I'm going to close this, but ideally as a follow-up it would be good to resolve this on the Three.js level so that all of the initialization can be moved to useState safely.

Three.js proposal: mrdoob/three.js#20575

it's confusing. Is there an option to disable it?

it's confusing. Is there an option to disable it?

It's hard to say what is confusing and what you want to disable. Strict Mode is opt-in, so yes, you can disable it by removing it from your app.

The behavior itself is intentional.

Navigation Menu

Repository navigation

In StrictMode, the useState() initializer function is called twice, but one of the results is discarded #20090

Steps To Reproduce

old demo

32 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

In StrictMode, the useState() initializer function is called twice, but one of the results is discarded #20090

Description

Steps To Reproduce

old demo

Activity

gaearon commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

drcmda commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

drcmda commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

drcmda commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

drcmda commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

gaearon commented on Oct 24, 2020

32 remaining items

gaearon commented on Oct 25, 2020

drcmda commented on Oct 25, 2020

drcmda commented on Oct 25, 2020

gaearon commented on Oct 25, 2020

drcmda commented on Oct 25, 2020

gaearon commented on Oct 25, 2020

gaearon commented on Oct 25, 2020

drcmda commented on Oct 25, 2020

gaearon commented on Oct 25, 2020

gaearon commented on Oct 25, 2020

gaearon commented on Oct 26, 2020

gaearon commented on Oct 28, 2020

FrankFang commented on Mar 25, 2021

gaearon commented on Mar 30, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions