styled-components: React.StrictMode throws warning because of `findDOMNode` use

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

I am aware that it is only run in development-mode, but it clutters console output when trying to find errors.

@Fer0x Could we change this here somehow? (by not using findDOMNode)

I don’t think it is a problem, not even sure if there is a proper solution. Keeping it for later reference.

_Originally posted by @optunetobi in https://github.com/styled-components/styled-components/pull/2073/review_comment/create_

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 39
  • Comments: 62 (30 by maintainers)

Most upvoted comments

Big thanks to @Fer0x for actually explaining what styled-components is doing with findDOMNode and why — which is missing from this thread and made it difficult to respond to it initially. For future cases where you feel React is getting in your way I’d appreciate if you could provide similar explainers. This was very helpful.

That’s issue can be reproduced when custom React component is passed to styled() factory. In this case styled-components create className and pass it as prop to that custom React component. If component is not supposed of supporting className prop, warn of incorrect usage should be thrown. So, in styled-components code, we need to find DOM node with generated className.

This approach seems a bit flawed to me because the general expectation is that it should be valid for a child component to not render the node with a class name (e.g. due to some early exit condition) and maybe render it later. So it’s a heavy handed assumption that I must render that className regardless.

Of course I did not design SC API and don’t have context on your decisions — I’m just looking from the point of view of a React component user. I generally expect that I can add return null into any component at any point in time and it shouldn’t cause me to see warnings. It’s a valid React feature.

function Story({ className, story }) {
  if (story.isLoading) {
    return null; // This shouldn't cause a warning but seems like it would?
  }
  return <div className={className} />;
}

Fragments present a different issue (findDOMNode will only find the first node) that can also cause misleading warnings. So I think we can agree the warning seems to stand in the way of using normal React features somewhat.

Now, maybe the mistake is very common and so the benefits of having a warning outweigh the false positives and other issues. I would argue that the issue might be inherent to this API that “pretends“ to override component’s styles with styled(Button) and might give people the wrong mental model. But maybe that’s also part of the appeal of styled components — it feels more familiar than e.g. CSS-in-JS, and styled(Button) kinda feels like selector overrides.

One thing you could do is to make className an object with custom toString that returns a class name in DEV. React would (AFAIK) toString it before rendering. That would let you detect that toString was called. If it’s not called within some timeout, you could warn in DEV. In production you could still use strings.

Yet another thing you could is to add a unique identifier to the className in DEV. (So, effectively, it would be two class names — one for the component and one for the instance — separated by spaces.) Then you could do the document.querySelector check suggested in https://github.com/styled-components/styled-components/issues/2154#issuecomment-444329164 without worrying about conflicts. Seems like that would be the easiest fix unless I’m missing something.

Hope that helps. As always, we’re happy to support you and try to find solutions to problems you encounter. But since we don’t use styled-components it’s difficult to understand the challenges you’re running into until we get a nice explanation like https://github.com/facebook/react/issues/14357#issuecomment-444045535. Instead of arguing whether the API needs to be brought back or not I think it would be much more productive if we always frame a discussion about a specific use case you need to solve.

No solid proposals have been raised for a replacement that handles all the same use cases.

Please file issues for those use cases. No solid proposal will be raised if those use cases are only described in other repos, and nobody comes to file them to ours. 😃

I think we will drop findDOMNode only when ref for Fragments be implemented. See https://github.com/facebook/react/pull/13841#issuecomment-430066195

Disabling strict mode will remove the warning messages, You can find the tag in index.js file .

Honestly I wouldn’t be surprised if they decide to undeprecate it eventually. No solid proposals have been raised for a replacement that handles all the same use cases. On Thu, Nov 15, 2018 at 1:26 AM Viktor Havrylin notifications@github.com wrote:

@zaguiini https://github.com/zaguiini Deprecating of findDOMNode in Strict mode is not only our pain. Third party libraries actively use it. For example: mui-org/material-ui#13394 https://github.com/mui-org/material-ui/issues/13394, jasonslyvia/react-lazyload#209 https://github.com/jasonslyvia/react-lazyload/issues/209, akiran/react-slick#1406 https://github.com/akiran/react-slick/issues/1406, reactjs/react-transition-group#429 https://github.com/reactjs/react-transition-group/issues/429, necolas/react-native-web#1151 https://github.com/necolas/react-native-web/issues/1151

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/styled-components/styled-components/issues/2154#issuecomment-438943519, or mute the thread https://github.com/notifications/unsubscribe-auth/AAiy1kJME2LZjwHdCEDZNTzg-mWmfjwKks5uvRcwgaJpZM4X6Dov .

StrictMode is enabled by default when using ReactDOM.createRoot and/or React.ConcurrentMode.

I’m open to a PR just removing this dev check

@manspaniel if you don’t want deprecation warnings then you shouldn’t use StrictMode. That defeats the whole purpose.

Unless you’re using React in Concurrent mode.

You shouldn’t be though. It’s not stable. Less stable than Hooks, even.

Getting this when trying to use styled components within WordPress v5’s new editor, which uses React, since their editor is wrapped in StrictMode 🤦‍♂️

Until either WP remove StrictMode, or styled-components is updated to use refs, I’ve been using this little hack to prevent the error from taking up 50 lines of my console each reload. Obviously not a nice one, but it works. Just thought I’d share 👍

;(() => {
  const oldLogError = console.error
  console.error = function(...args) {
    if (typeof args[0] !== 'string' || args[0].indexOf('is deprecated in StrictMode') === -1) {
      oldLogError.apply(console, args)
    }
  }
})()

It looks like you’re using findDOMNode only for a DEV-time warning. Is that warning really so valuable?

@optunetobi I have tried to replace findDOMNode with ref. I made it like it described in following issue https://github.com/facebook/react/issues/13029, but it doesn’t work for cases with functions like styled(() => <div />) which are main target for classNameUseCheckInjector warning. So this issue needs further investigation.

What if the check were just slightly less focused and did document.querySelector(selector) to see if any DOM node contains the class name that styled-components generated? I think the only issue would be potential false negatives (missed warnings) if another styled element on the page ends up using the same generated class name, thus satisfying the check, right? Or maybe the issue is components that suspend or render null in certain states, so you want to make sure they rendered a DOM node at all first?

Unless you’re using React in Concurrent mode.

You shouldn’t be though. It’s not stable. Less stable than Hooks, even.

Not doing in production and please, whomever is doing: stop it. I was just messing around and found the warning 🤷‍♂️

Which is also optional 😃

There is a cost to using new features before they’ve had time to mature.

Well, if there was a way to disable StrictMode, people would do it, wouldn’t they? xD

Thanks anyway!

Yeah I mean StrictMode is optional so just don’t use it until a replacement API is available 🤷‍♂️