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

[drcmda]

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.

[gaearon]

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?

[gaearon]

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

[gaearon]

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.

[drcmda]

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.

[gaearon]

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

[drcmda]

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

[gaearon]

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.

[drcmda]

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.

[gaearon]

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.

[drcmda]

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.

[gaearon]

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.

[gaearon]

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.

[gaearon]

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.

[gaearon]

Here's something I don't understand in particular.

I see you're rendering <OrbitControls enableZoom={false} />. But that props.enableZoom value is never being passed to the OrbitControlsImpl constructor. Instead, you pass it down to <primitive>: <primitive object={controls} {...props} />.

The code that's logging in OrbitControls useEffect is logging controls.enableZoom, not props.enableZoom. So why do you expect that controls.enableZoom would be false? You never set it from props, and the thing initializing it is inside the OrbitControlsImpl code (this.enableZoom = true).

[gaearon]

Here is a version where I added a getter/setter for controls.enableZoom to understand what is setting it.

https://codesandbox.io/s/r3f-contact-shadow-forked-u9g0b?file=/src/OrbitControls.js:146-338

  Object.defineProperty(this, 'enableZoom', {
    get() {
      return this._enableZoom
    },
    set(val) {
      this._enableZoom = val
      warn('setting enableZoom to', val)
    }
  })

Now let's see what happens.

First, the state is initialized and the constructor runs.

It has side effects (not allowed), but let's forget about that for a moment. It sets this.enableZoom = true in the constructor.

Next, the constructor runs again. The state initializer runs again.

Again, side effects (not allowed), but that's not the point. This one also sets this.enableZoom = true in the constructor.

So by this point both of them have enableZoom = true.

Finally, I'm seeing this assignment!

OK, so this is why one of them gets enableZoom = false. It's the renderer itself (R3F) that mutates an external object passed into it inside createInstance. Since only the result of one of the renders is used (the other is ignored), only one of these objects gets passed into the renderer for the <primitive>, and thus only one of them gets mutated.

I'd say it's shady that createInstance would mutate anything passed to it, especially a prop. Props should never be mutated in React at all. Maybe there's some cases where this can work without problems but just like ReactDOM or React Native never mutates props passed to createInstance, I would expect R3F to be able to not do that.

Anyway, what happens next makes sense now.

We've got two listeners (because of a subscription during render, which is not allowed and has always been a problem for server rendering, so is not idiomatic), both of which are active. One of them has .enableZoom mutated by the renderer (which is bad because renderers shouldn't mutate props passed to them). Another doesn't, because the result from that render was discarded (and so not passed to your custom renderer, which therefore didn't mutate it).

Hence the issue.

Does this make sense? We can brainstorm some alternative solutions but I'd like to verify if I understood the issue correctly.

[gaearon]

This piece of code in R3F looks like createInstance doesn't actually create an instance. It may be reusing one passed as a prop.

This is not a supported pattern — the method name literally starts with "create". I hope you can understand that while I want to help figure out a solution here, it is tough when I can't assume that the renderer code respects the contract. Of course, any changes to React can accidentally break it if it does something as clever as reuse passed instances instead of creating them.

[gaearon]

why does concurrent mode create two controls?

By the way. I want to briefly answer the concrete question, i.e. why is it even useful to check for this.

One reason is related to server rendering. By double-rendering the initialization sequence, we expose memory leaks and bugs that would have otherwise occurred only on the server, on the client. I understand that you don't personally need server rendering since R3F is client-only. But for many ReactDOM users, at some point the ability to render their app to HTML becomes an extremely important performance optimization, especially for low-end devices. We don't want to put people in a situation where they turn on server rendering, and half of their components are completely broken because they do subscriptions and other side effects during the initial render. So this is one part of the motivation.

The other part is that we need to assume render can be "thrown away" for many future features. For example, consider Suspense. My impression was that you've found it fairly useful in R3F. However, Suspense relies on being able to "throw away" a rendered tree midway if some part of it is not ready, and then trying to render it again later. For this to work, you need to be able to safely initialize state, throw it away, and then initialize it again. You can't just reuse the old result because the props might have changed by the time you do a second attempt. This is why the render phase needs to be pure (including state initializers). This is not specific to Suspense. For example, if React adds built-in animation and gesture low-levell primitives, it would also be important to be able to render "start" and "future" states virtually to generate keyframes between them. This can't work if the process of rendering itself performs side effects. So this is what we tease out by doing double render.

[drcmda]

yes, primitive is an integral part, allows you to project an already existing thing into the react world. the only difference is that it is created beforehand, there's no real conflict here. for this particular platform imperatively created objects are a given come what will.

What is happening is this:

1. OrbitControls are created and stored in useState
2. React interrupts the render phase, for some reason, and makes a full reset, it does not get to the view part where the enableZoom prop is set
3. The component restarts from scratch, another OrbitControl is created
4. This time React reaches the view, in which the prop is set
5. Now 2 instances catch mouse events, the first one has zoom enabled, the second doesn't

Therefore i'd like to ask for a more reasonable means to handle local state

Blocking mode:

function OrbitControls(props) {
  const { camera, gl } = useThree()
  const [controls] = useState(() => new OrbitControlsImpl(camera, gl.domElement))
  useFrame(() => controls.update())
  return <primitive object={controls} {...props} />
}

Concurrent mode:

function OrbitControls(props) {
  const { camera, gl } = useThree()
  const [controls, set] = useState()

  useLayoutEffect(() => {
    const temp = new OrbitControlsImpl(camera, gl.domElement)
    set(temp)
    return () => temp.dispose()
  }, [])

  useFrame(() => {
    if (controls) controls.update()
  })
  return controls ? <primitive object={controls} {...props} /> : null
}

This is cumbersome and it will hit every single time you're dealing with side-effects in local state, which may or may not be evident since you have no control over 3rd party controls. In the official react docs you use Remarkable - how do you know that it doesn't set some global variable or does something else that's out of the ordinary?

A suggestion: useEffectState

Could there be a hook that abstracts this? Something that will become the official means of dealing with local state - if it has side effects or not. With a clean-up phase, but immediate return.

const controls = useEffectState(
  () => new Foo(),
  previous => /* cleanup */, 
  [conditions]
)

console.log(controls) // it's immediately available

[gaearon]

I think there is some misconception that I want to get to the bottom of.

React interrupts the render phase, for some reason, and makes a full reset, it does not get to the view part where the enableZoom prop is set

although i do understand it must be able to just break in the midst of render.

React doesn't "break in the midst of render". This is technically impossible. React can't make your code stop in the middle.

Instead, we need to be clear that JSX does not produce immediate function calls.

When you see

render() {
  return <MyComponent />
}

It doesn't mean that MyComponent gets called during render. This is not how React works, and it has never worked this way. <MyComponent /> becomes a plain object like { type: MyComponent, props: {} }.

So what happens here isn't that React magically stops your code from executing. It just means that it calls your component twice:

// somewhere in React
YourComponent(); // ignore the result
let result = YourComponent(); // use the result

This behavior has been on in StrictMode since 2018, so by itself it is not new. And the goal of it, as I explained earlier, is precisely to catch side effects during initialization or render — which it did, as designed.

(I don't know why you didn't see the issue with old versions, but if you make a CodeSandbox with the right versions that don't show the problem even in StrictMode, I can dig into this.)

Does this explanation make sense? We can talk more about specific solutions or whether the "reusing" createInstance pattern is valid, but I want to check if we're on the same page that your render function doesn't get "interrupted".

[drcmda]

sure, i can, here's a version on the previous reconciler and react 16.4 in concurrent mode: https://codesandbox.io/s/r3f-contact-shadow-forked-z93o3

i get why this is no solution, even if that old reconciler worked. in a way im glad this comes up now. this must have happened a thousand times silently without anybody noticing.

i think a new hook or anything that spares us the useLayoutEffect soup will go a long way in making this easier. when you see the useEffectState above, could that be a way?

[gaearon]

when you see the useEffectState above, could that be a way?

I don't think it could be a way. But it's hard to explain why because I'm not sure that we have alignment even on simpler things (e.g. whether or not your render function got interrupted). I was wondering whether my previous comment makes sense and whether you see what I meant by that.

[drcmda]

i think we're on the same page. though i still don't fully understand why one instance of the orbitcontrols does not get that flag then. if react does not interrupt - how can it end up without commitUpdate/applyProps. but either way, the internals are messing with my head, i better leave that to the react team.

all that matters to me is that we have a sane way of dealing with local side-effect state, which currently seems only possible by jumping through hoops, rendering twice, checking against undefined.

[gaearon]

sure, i can, here's a version on the previous reconciler and react 16.4 in concurrent mode: https://codesandbox.io/s/r3f-contact-shadow-forked-z93o3

From what I'm seeing here, R3F is compiled in production mode (despite the development environment). For this reason, it doesn't have very important warnings (e.g. about duplicate keys), and the StrictMode double-rendering is disabled.

I'm guessing you fixed this in the latest release, which is why the behavior is showing up now.

[drcmda]

Ah yes, that was our discussion on twitter. It is inlining react-reconciler in prod mode there.

[gaearon]

but either way, the internals are messing with my head, i better leave that to the react team.

I think you're overestimating the complexity of internals in this particular case. If you're creating a third-party renderer, it is helpful to have the right high-level mental model. Let me try to walk through it and see if that makes sense?

if react does not interrupt - how can it end up without commitUpdate/applyProps.

Say this is your component:

function YourComponent() {
  return <div />
}

Imagine this is React:

YourComponent(); // ignore the result
let result = YourComponent();

let domNode = rendererConfig.createInstance(result.type, result.props);

I think this shows why even if YourComponent is called twice, React would still call createInstance only for one of those attempts, and this doesn't violate JS rules. Does this make sense?

When you write <div /> it doesn't call any renderer methods. It doesn't create a DOM node. It's just a plain object — an instruction to the renderer. The renderer executes instructions from one render attempt, but the spurious one is intentionally thrown away and ignored.

[drcmda]

ok, i get it now. and applyProps is within createInstance. that's why it ends up like that. how would i solve it, given that i must allow something like <primitive/> in order to maintain compat with the host.

[gaearon]

OK, glad we're on the same page.

Now regarding solving this... I understand that you already rely on this behavior and it's frustrating. But I do want to point out that if we were talking about anything similar related to ReactDOM, we would have the same problem with server rendering. It is simply not supported to have side effects during render because it limits the kinds of features we can build into React. It breaks server rendering, error handling, and other cases.

In the official react docs you use Remarkable - how do you know that it doesn't set some global variable or does something else that's out of the ordinary?

We don't — you can never really know that in JavaScript. It is a matter of convention. For years, when people run into this with an imperative library, they would either work around it by initializing in componentDidMount (aka useLayoutEffect) or send a pull request to the imperative code that would separate initialization from side effects. Again, I totally understand you're not particularly keen to contribute to the Tree.js codebase to solve this, but this is how it's been solved for years when such libraries are used from ReactDOM apps.

Arguably, side effects in constructor are widely considered to be a bad practice, regardless of whether you use React or not. It's worth fixing that imperative code to separate the event subscription from the rest of the initialization. Or at least offer an opt out.

Sure, we can talk more about possible workarounds, but I want to be clear that the idiomatic solution is to solve this at the Three level.

[gaearon]

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.

[drcmda]

@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.

[drcmda]

@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])

[gaearon]

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.

[drcmda]

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

[gaearon]

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.

[gaearon]

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.

[drcmda]

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.

[gaearon]

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.

[gaearon]

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?

[gaearon]

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.

[gaearon]

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

[FrankFang]

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

[gaearon]

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.