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

StrictMode warning for findDOMNode and legacy context API in Transition as of React 16.6 #429

Closed
TAGraves opened this issue Oct 25, 2018 · 25 comments · Fixed by #559
Closed
Labels

Comments

@TAGraves
Copy link

Do you want to request a feature or report a bug?
Report a bug

What is the current behavior?
When using <Transition /> inside of <React.StrictMode />, warnings are logged about findDOMNode and the legacy context API (as of React 16.6)

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via CodeSandbox.
View the console at https://codesandbox.io/s/y0r3kl6x1j. On mount, you'll see:

Warning: Legacy context API has been detected within a strict-mode tree

and after clicking "Toggle in Prop", you'll see:

findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Transition which is inside StrictMode. Instead, add a ref directly to the element you want to reference.

What is the expected behavior?
No warning should be logged.

Which versions, and which browser / OS are affected by this issue? Did this work in previous versions?
These warnings are new as of React 16.6.

@jquense
Copy link
Collaborator

jquense commented Oct 25, 2018

Thanks! I wouldn't consider this a bug since it's a deprecation warning but we'll do what we can to use refs if feasible

@jquense jquense closed this as completed Oct 25, 2018
@edmorley
Copy link

edmorley commented Nov 9, 2018

Please can this be reopened until it's fixed? :-)

The warning makes using <StrictMode> very noisy, which effectively blocks us using it to prevent other issues.

@ekohanyi
Copy link

This warning has now stopped transitions from working in strict mode, not just showing in the console but continuing to operate. So it makes it difficult or impossible to add new animations without turning off strict mode on local environments.

react: v16.6.3
react-transition-group: v2.3.1

@jquense
Copy link
Collaborator

jquense commented Dec 21, 2018

The warning can't stop transition s it's just a console.log not a thrown error

@ekohanyi
Copy link

It might be something else but we noticed the transitions weren't firing and then when we turned off strict mode it started animating properly again, so seems like something is getting stopped up related to it

@Jessidhia
Copy link

StrictMode doesn't only print warnings, it also double-renders all components in development.

@ekohanyi
Copy link

@Kovensky Ohh okay, good to know, that must be what's causing the issue then. Thank you! I learned something new today haha

@Iverson
Copy link

Iverson commented Jan 24, 2019

Is there any progress here? It's really painful to work with console in <StrictMode>.

@sseppola
Copy link

I haven't been able to wrap my head around all the considerations this lib takes, but to move the conversation forward:

1) Are there any specific challenges to use the new context api?

From what I could gather by browsing the source code, maybe it could cause problems for nested transitions? (https://github.com/reactjs/react-transition-group/blob/master/src/Transition.js#L150)

The Transition would have to consume the context via a render callback, so maybe we could just wrap the exported component so that it would receive transitionGroup as a prop instead, eg:

export default function (props) {
  return (
    <TransitionContext.Consumer>
      {(transitionGroup) => <Transition {...props} transitionGroup={transitionGroup} />}
    <TransitionContext.Consumer>
}

Are there performance considerations I'm missing? Backwards compatibility with React 15?

2) Are there any specific challenges to use refs in this case?

I couldn't spot anything right now.

@thaggie
Copy link

thaggie commented Apr 11, 2019

This pull request should fix the findDOMNode warning - #486

jquense pushed a commit that referenced this issue Apr 15, 2019
`React@16.6.0` allows usage of static `contextType`. With `16.0.0` we would need a higher-order component that puts the context value into props since `Transition` needs to have access to the context in its constructor.

Partial fix for #429  

BREAKING CHANGE: use new style react context

```diff
// package.json
-"react": "^15.0.0",
+"react": "^16.6.0",
-"react-dom": "^15.0.0", 
+"react-dom": "^16.6.0", 
```
jquense pushed a commit that referenced this issue Apr 15, 2019
# [3.0.0](v2.9.0...v3.0.0) (2019-04-15)

### Features

* use stable context API ([#471](#471)) ([aee4901](aee4901)), closes [#429](#429)

### BREAKING CHANGES

* use new style react context

```diff
// package.json
-"react": "^15.0.0",
+"react": "^16.6.0",
-"react-dom": "^15.0.0",
+"react-dom": "^16.6.0",
```
@ybiquitous
Copy link
Contributor

@jquense Can you close this issue?

@hlehmann
Copy link

There is still the findDOMNode issue and using ForwardRef would cause a breaking change, nonetheless React.ConcurrentMode will require it.

@chybisov
Copy link

chybisov commented Nov 1, 2019

Is there any progress here after one year? It's still really painful see these errors in <StrictMode>

@JSteunou
Copy link

#514 might be a could transitional solution, please consider it :)

@IrvingArmenta
Copy link

IrvingArmenta commented Feb 13, 2020

Is there any progress here? It's still really painful see these errors in

@3limin4t0r
Copy link

For those annoyed by the warning in strict mode. If you don't use the newest React features you could lower the React version to 16.5. The warning is introduced in version 16.6.

@silvenon
Copy link
Collaborator

silvenon commented Apr 8, 2020

We're looking for a workaround for this, feel free to voice your opinions in the RFC: #606

@jquense
Copy link
Collaborator

jquense commented May 5, 2020

🎉 This issue has been resolved in version 4.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@alecmarcus
Copy link

alecmarcus commented May 9, 2020

Seems the issue may still be present with SwitchTransition.

I do have 4.4.1 installed via npm.

index.js:1 Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an 
instance of Transition which is inside StrictMode. Instead, add a ref directly to the element you 
want to reference. Learn more about using refs safely here: 
https://fb.me/react-strict-mode-find-node
    in div (created by About)
    in About (created by Context.Consumer)
    in Route (created by D)
    in D (created by Context.Consumer)
    in S (created by Context.Consumer)
    in S (created by Context.Consumer)
    in Route (created by k)
    in k (created by Routes)
    in Switch (created by Routes)
    in Transition (created by CSSTransition)
    in CSSTransition (created by Routes)
    in SwitchTransition (created by Routes)
    in section (created by Default)
    in main (created by Default)
    in section (created by Default)
    in Default (created by Routes)
    in Switch (created by Routes)
    in j (created by Routes)
    in Routes (created by App)
    in App
    in Router (created by BrowserRouter)
    in BrowserRouter
    in ApolloProvider
    in StrictMode```

@pmoons
Copy link

pmoons commented May 9, 2020

@alecmarcus Are you using the the new nodeRef prop that was introduced in version 4.4.0? Using the prop for me removed the console error.

Short write up about this prop in the CHANGELOG

@alecmarcus
Copy link

@pmoons Wonderful, that was it (my bad for not reading). Thanks for the help!

@evoyy
Copy link

evoyy commented Jun 4, 2020

I appreciate the work that has been done on this package, but this issue should not be closed. After upgrading to 4.4.1 I still get the warning when I enable strict mode. I saw the changelog entry that explains the workaround of passing a nodeRef prop. However, the default behaviour of this package still produces the warning in strict mode and hence this issue is not technically resolved, only a workaround has been provided.

@silvenon
Copy link
Collaborator

silvenon commented Jun 7, 2020

It's not a workaround, you have to explicitly assign a node to react-transition-group, otherwise it cannot avoid using findDOMNode. Read the official explanation for deprecating findDOMNode to understand why.

We would've loved to solve this without people having to update their code, but it's just not possible.

@LucaPizzagalli
Copy link

I think the documentation should be updated, too.
This example produces the "findDOMNode is deprecated in StrictMode" warning, and it's still not clear to me how to solve the issue.

@silvenon
Copy link
Collaborator

silvenon commented Jun 14, 2020

They are most probably using the outdated version of react-transition-group, so you have no control over that. I wouldn't suggest for them to upgrade yet, though, we complete the roadmap described in #630.

Regarding the documentation, please describe what specifically you believe needs to be updated in a new issue.

@reactjs reactjs locked as resolved and limited conversation to collaborators Jun 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet