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

Disable react strict mode on third party libraries #16362

Closed
israelKusayev opened this issue Aug 12, 2019 · 49 comments
Closed

Disable react strict mode on third party libraries #16362

israelKusayev opened this issue Aug 12, 2019 · 49 comments

Comments

@israelKusayev
Copy link

I started to use react strict mode in my project.

But I'm using third party libraries (like old react-bootstrap).

Is it possible to disable the strict mode on those libraries specifically, or disable strict mode to all third party libraries?

@israelKusayev
Copy link
Author

Also for the new update is it possible to disable the "Rename componentWillMount" messages to third party libraries?

@threepointone
Copy link
Contributor

threepointone commented Aug 12, 2019

If React made these exceptions, then it wouldn’t be “strict” at all :) so no, I don’t think React will skip third party libraries for warnings.

Could you tell me why you’re using strict mode if you cannot commit to the guarantees just yet? I would recommend you not use third party libs that aren’t strict mode compatible. Alternately, don’t use strict mode if your project isn’t ready for it.

@israelKusayev
Copy link
Author

It's a big project so I cannot update the libraries that we are using.
But we still want to update react version and /or using strict mode for our code.

If react team can add the ability to disable strict mode for those libraries it would be great

For example

  <React.StrictMode thirdParty={false}>
       // code goes here
  </React.StrictMode>

@threepointone
Copy link
Contributor

React can’t detect which components are “third party”. These other libraries should release versions which either rename the method names, or refactor with the suggestions in the warning message. Based on github activity, I notice a lot of libraries are already doing this.

Is there anything critical for you in the 16.9 release that warrants an upgrade of React? I would consider the task of upgrading React libs to be dependent on that.

@gaearon
Copy link
Collaborator

gaearon commented Aug 12, 2019

You can always put <StrictMode> more granularly than around the whole app. Start from some smaller sections that don't contain outdated third party components, and gradually move it upwards as they get fixed.

We don't currently offer a more granular way to opt into strict mode. But keep in mind is that its whole purpose is to tell you whether the app is compatible with future planned changes. So if you silence it for third party components, you don't gain that much in terms of guarantees.

@gaearon gaearon closed this as completed Aug 12, 2019
@hatton
Copy link

hatton commented Feb 8, 2020

So if you silence it for third party components, you don't gain that much in terms of guarantees.

Right, but while we're waiting for 3rd party components to catch up, it is very nice to immediately catch if our code starts violating anything.

You can always put more granularly than around the whole app.

Thanks for that suggestion, that's what we're doing for now, but you can't mark a component as strict if any of its descendants are not, so it's of limited use. I assume it wouldn't be feasible to instead be able to fence off sections that are not yet strict, so that they are easy to find? E.g.

<StrictMode enabled={false}> or <BreaksStrictMode>

That way we could put much more of the code under the Strict checking.

@MaximilianLloyd
Copy link

Any update on this?

@gaearon
Copy link
Collaborator

gaearon commented Nov 20, 2020

Why would you expect an update on this?

There are no plans to support more granular strict mode. The purpose of the strict mode is to offer a guarantee about the behavior of the subtree. I very much understand the temptation for partial guarantees but that undermines the whole principle. With this behavior when you look at StrictMode you wouldn’t know whether the subtree is strict-compatible, or something else overrides it two components below. So this makes the feature useless as the project grows.

@starikcetin
Copy link

This would be very useful indeed. 3rd part libraries are cluttering my console output with all sorts of strict mode warnings.

@epikhighs
Copy link

3rd party libraries are outside of my direct control so I wouldn't want to be barraged by warnings I can't do anything about. The only thing to do for 3rd party library warnings is to wait and see if there'll be a fix or switch the library for an alternative (assuming a suitable alternative exists).

The other issue is that strict mode seems to break functionality.

I'd love to keep using strict mode, but having to manually include every component except for the one component isn't very practical (at least for me).

@bvaughn
Copy link
Contributor

bvaughn commented Jan 12, 2021

The other issue is that strict mode seems to break functionality.

To me, that indicates that Grid has a bug and StrictMode is making the bug's behavior occur in a reproducible way. That's one of the main purposes of strict mode and how it works.

@epikhighs
Copy link

Yea, I understand that, which is great for the author of the 3rd party lib, but not much I can do about it other than disable react strict mode for my entire app :)

@bvaughn
Copy link
Contributor

bvaughn commented Jan 12, 2021

Which is probably what you should do (at least for now). StrictMode is providing a valuable signal in this case: the library you're using in your application is probably not going to work well in concurrent mode.

@chriscummings100
Copy link

I'll chip in here, as it would be really helpful for us too. In answer to the comments about 'you should either have it for the entire tree or not at all', it's very similar to the classic 'warnings as errors' debate in c++ compilers. Ideally you should fix all warnings in your code, as they are there for a reason. However sometimes there are warnings you fundamentally can't fix or disable.

You could argue that you should just tolerate warnings for those areas of code - they are there for a reason after all. However if lots of warnings are pumped out, they start to become noise, and human developers in the real world begin to miss/ignore genuine warnings.

A real world old school example I had on a team was implementing games that included the Nintendo Wii UI, which was a big set of c++ files provided by Nintendo packed full of warnings. Our whole code base had 0 warnings in except for that library. We couldn't modify it, as it was strictly enforced by the 3rd party, who had no intention of fixing it. In that scenario, the best solution for the team and the project was to disable warnings for the Nintendo code, so our team code focus on ensuring the rest of our code had 0 warnings.

This one is similar - in our case it's the bootstrap modal dialog. If every time it pops up you see a flood of errors, you stand a good chance of missing other errors in your own code. Counter intuitively, disabling those errors until we have a fixed bootstrap dialog will help us reduce bugs in our project, as genuine errors will be more obvious.

I understand that's not a solution for everybody, and some may prefer to be more strict. But for us and many other teams, we feel a simple <React.DisableStrictMode> to use in the worst case scenarios would improve the team's ability to catch bugs and reduce errors. Ultimately it should be a decision for the team, who understand their own use case the best.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 28, 2021

We agree with the concerns that a flood of errors/warnings masks any single type of error (even one that may be more pressing) and teaches people to just ignore errors/warnings.

But let's take a step back and consider why you're using <StrictMode>. In case it's not clear, no one needs to use it. It exists for one very specific reason: to provide a signal about whether the part of an application it wraps around contains code that is likely to cause problems in concurrent mode (aka our new createRoot API). If you are not planning to use createRoot in the near future then <StrictMode> doesn't add a lot of value to your application and you can just remove it for now.

Our team talks about things like this often so there's always a chance that we may reevaluate this decision (either the planned semantics of createRoot or the option to opt out of warnings for part of the subtree). We'll keep everyone posted if/when anything changes.

@chriscummings100
Copy link

Well, if your honest answer is that we should simply turn it off, then I guess we can follow it. I'll be honest though, it sounds suspiciously like the line "why don't we just disable warnings as errors", which I've heard about 1000 times over my years as a TD (and at one point started a swear box for programmers that even suggest it!).

@bvaughn
Copy link
Contributor

bvaughn commented Jan 28, 2021

Yes, my advice is to turn it off for now if:
(1) it's causing a lot of warning/error messages that are interfering with your daily development (particularly if a lot of them are not actionable because they are in external libraries) and especially if
(2) you aren't planning to start using the new createRoot API in the near future.

Perhaps we'll come up with improvements to the suggested upgrade path before we release createRoot as stable. We're always thinking about this.

@chriscummings100
Copy link

Will do - thx. If you have a chance, I'd appreciate it if you manage to notify this thread if things change.

@dan003400
Copy link

How do you deal with the findDOMNode issue when working with things like PayPal Checkout?

https://developer.paypal.com/docs/checkout/#add-the-buttons-to-your-website

@OnkelTem
Copy link

OnkelTem commented Mar 1, 2021

Actually, we disable strict mode on all our projects. We don't see any real benefits from it, only problems.

@SohanChotia
Copy link

@OnkelTem How do you disable strict mode in a RN app, I m new to it and it is caugin multiple ajax calls in components.
Please help

@OnkelTem
Copy link

OnkelTem commented Mar 9, 2021

@SohanChotia

Just don't enable it. It's usually enabled in the index.ts by surrounding your topmost component (<App>) with <React.StrictMode> tags.

https://reactjs.org/docs/strict-mode.html

@graup
Copy link

graup commented Oct 28, 2021

A way to mark sections that have expected side effects, something like <BreaksStrictMode>, would be great.

Some code just HAS to break the side effect guarantee, for example tracking code. It's difficult to test tracking code if it gets fired twice.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 28, 2021

Some code just HAS to break the side effect guarantee, for example tracking code. It's difficult to test tracking code if it gets fired twice.

Why is tracking code special wrt side effects? Tracking should be done in an effect. (You wouldn't want to log an impression for a component that rendered but never mounted - aka was never visible to the user)

@graup
Copy link

graup commented Oct 28, 2021

Some code just HAS to break the side effect guarantee, for example tracking code. It's difficult to test tracking code if it gets fired twice.

Why is tracking code special wrt side effects? Tracking should be done in an effect.

Hmm, I had it in an effect (with an empty dependency list) but it still got called twice. Maybe I still misunderstand what StrictMode does and my issue is a different one.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 28, 2021

Ah. That's something different then. This thread was mostly about strict mode (double rendering). You're referring to "strict effects" which is a newer thing.

You might want to check out this Working Group series on "strict effects". Or just scan for the "effects that should only run once can use a ref" header in this post.

tl;dr is to use a ref:

const didLogRef = useRef(false);

useEffect(() => {
  // In this case, whether we are mounting or remounting,
  // we use a ref so that we only log an impression once.
  if (didLogRef.current === false) {
    didLogRef.current = true;

    SomeTrackingAPI.logImpression();
  }
}, []);

If you want to understand the background reasoning/motivation for this behavior though, the above posts will be helpful.

@snawaz
Copy link

snawaz commented Apr 10, 2022

I wish I could do something like this:

<React.StrictMode>
       <App>
             ..
             ..
             ..
              <React.NonStrictMode>
                  // code that gives warnings/errors in strict mode
                  // e.g some component of third-party libraries like blueprintjs (even the latest version) 
              </React.NonStrictMode>
             ..
             ..
             ..
       </App>
  </React.StrictMode>

That is, disable strict-mode for a tiny subtree and enable everywhere else.

@gaearon
Copy link
Collaborator

gaearon commented Apr 10, 2022

Yeah. We considered adding this, but there’s too high a risk that some third-party dependency will include it in its tree and then everything below (whether from that library or not) will lose strictness.

@snawaz
Copy link

snawaz commented Apr 11, 2022

@gaearon

Yeah. We considered adding this, but there’s too high a risk that some third-party dependency will include it in its tree and then everything below (whether from that library or not) will lose strictness.

But that is what the authors of the library want. So why not allow it? The whole purpose of adding such a feature is to disable warnings from code which the devs cannot fix because:

  • either the warnings are coming from third-party libraries,
  • or from some code which the authors do not want to touch at the moment.

Also, as per the docs: Strict mode checks are run in development mode only; they do not impact the production build. So the risk is in the development build only, not in the production build.

@gaearon
Copy link
Collaborator

gaearon commented Apr 11, 2022

But that is what the authors of the library want. So why not allow it?

Because if a component accepts children (as many components do — and especially library components), it can’t possibly make claims about what’s put inside of it. This undermines the whole mode because you can’t be sure if anything in the middle has disabled the checks. (And the mistake could be added in any patch release.)

The whole purpose of adding such a feature is to disable warnings from code which the devs cannot fix

I believe we’re starting to go in circles because the response to this is similar to our earlier comments.

We think of Strict Mode as a tool to assert that a tree is compatible with new React features and surfacing bugs early. If you let a part of the subtree opt out, you lose the power of assertions. “This dish is safe to eat because all of the ingredients are safe” loses meaning if some ingredients can opt out of safety. This is similar.

So the risk is in the development build only, not in the production build.

This doesn’t sound right to me. The very purpose of Strict Mode is to surface bugs you’d have in production, but early. Yes, React won’t stress-test your app the same way in production. But the “risk” is in writing buggy components and not noticing because someone else has added a suppression anywhere in the tree.

@snawaz
Copy link

snawaz commented Apr 11, 2022

We think of Strict Mode as a tool to assert that a tree is compatible with new React features and surfacing bugs early. If you let a part of the subtree opt out, you lose the power of assertions. “This dish is safe to eat because all of the ingredients are safe” loses meaning if some ingredients can opt out of safety. This is similar.

Hmm. Sounds reasonable. So how do we have both without comprising? I guess introducing another mode would help, like <PartiallyStrictModeOn> and <PartiallyStrictModeOff>.

Anyway, the warnings from the third-party libraries are highly irritating — if there are too many of them, then the developers might miss the other important warnings that they can actually fix. That is a problem that actually exists. Practically, it isn't possible to fix all the warnings.

So with or without <StrictMode>, there is a risk:

  • If we dont use it, there is a risk of bugs.
  • If we use it, then it helps use to find the potential issues but it also gives too many warnings from third-party libraries as a result of which we might miss the important warnings, means there is a (different kind of) risk of bugs.

🤔

@gaearon
Copy link
Collaborator

gaearon commented Apr 11, 2022

Hmm. When talking about Strict Mode I primarily mean the double-rendering behavior. It seems like your primary issue is warning spam. Can you clarify which specifically warnings you are seeing most often? And how many do you get?

@snawaz
Copy link

snawaz commented Apr 11, 2022

Hmm. When talking about Strict Mode I primarily mean the double-rendering behavior. It seems like your primary issue is warning spam. Can you clarify which specifically warnings you are seeing most often? And how many do you get?

Yes. My primary concern is warning spam! I'm not a frontend developer, so I don't know much about React or web development in general.

I'm working on a personal app and React is super easy (all thanks to you and your team 😄 ). So I'm using it. The app is very small now, so I don't get too many warnings honestly speaking, still enough to irritate me (to be honest, even a single warning irritates me, HAHA, maybe because I take them very seriously). Anyway, here are the warnings coming from the latest version of Blueprintjs (I was using an older version earlier and upgraded yesterday thinking the latest version (v4) has fixed them, but as per my findings, the Blueprintjs team has decided to fix this in v5 instead).

findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Transition which is inside StrictMode.

More:

image

@bvaughn
Copy link
Contributor

bvaughn commented Apr 11, 2022

Anyway, the warnings from the third-party libraries are highly irritating — if there are too many of them, then the developers might miss the other important warnings that they can actually fix.

Just want to chime in and say that it's very understandable how frustrating it must be to see a bunch of warnings for things that you can't directly fix (because they're things in third party code). It can also be frustrating if the warnings are deprecation warnings, and you're not planning to upgrade in the near future.

The React team has a pretty high bar for adding warnings– only things that could potentially cause problems during an upcoming upgrade or in (production) code. Problems often originate in third party code. My concern about silencing those warnings is that it might lead to a false sense of security around how future-proof a part of your application is. Where as seeing those warnings hopefully leads to either a bug report or a PR – and eventually a new release of the library that fixes the underlying issue.

@gaearon
Copy link
Collaborator

gaearon commented Apr 11, 2022

These warnings seem important to fix because we will be removing these features (legacy context and findDOMNode) in a future major release completely. They have been deprecated inside of strict mode for several years by now, so we don’t see this very often in the ecosystem. The warnings are doing their job here because eventually, if you keep using libraries relying on these features, you will not be able to upgrade React. I empathize with the frustration here but over the last several years the ecosystem has largely fixed them, so suppressing them now doesn’t seem wise. They really need to be fixed if you don’t want to get stuck without future upgrades.

@snawaz
Copy link

snawaz commented Apr 11, 2022

I empathize with the frustration here but over the last several years the ecosystem has largely fixed them, so suppressing them now doesn’t seem wise.

Not suppressing them isn't useful either. It can be harmful instead.

Also, it's not about a particular warning. I'm talking about things in general. These are merely examples which demonstrate the idea that the devs cannot do anything here, as these are coming from third-party libraries and libraries like all softwares take some time to fix warnings, some could even be there in many new versions like in this case. Not all warnings are immediately fixed. It's true for all languages and all frameworks. Tools often provide a way to suppress warnings selectively if it helps developers.

@gaearon
Copy link
Collaborator

gaearon commented Apr 11, 2022

They’re actionable in the sense that — if you heed the warnings — you need to upgrade and/or replace the packages emitting them. Or, alternatively, remove the checks.

Tools often provide a way to suppress warnings selectively if it helps developers.

If you feel strongly about this, you can override console.error and filter out the messages manually. It’s crude but it works.

@snawaz
Copy link

snawaz commented Apr 11, 2022

They’re actionable in the sense that — if you heed the warnings — you need to upgrade and/or replace the packages emitting them. Or, alternatively, remove the checks.

Sometimes none works. That is my case.

If you feel strongly about this, you can override console.error and filter out the messages manually. It’s crude but it works.

ah, that's a very nice hack. Looks like it'll work in my case. 🤔 Thanks for the advice.

@emilienak
Copy link

My 2 cents to temporary silence known warnings from 3rd party libs.
Just concatenate some regex in your console Filter to exclude the offending warnings:
-/Warning.*findDOMNode.*withBoundingRects/
Tested in chromium based browsers.

@Nantris
Copy link

Nantris commented Sep 1, 2022

Ah. That's something different then. This thread was mostly about strict mode (double rendering). You're referring to "strict effects" which is a newer thing.

StrictMode does now include strict effects as of React 18, right?

Yeah. We considered adding this, but there’s too high a risk that some third-party dependency will include it in its tree and then everything below (whether from that library or not) will lose strictness.

I think that's a great point @gaearon. I can't imagine this would still be considered, but it seems like having a component to disable StrictMode for its children could be safe if it requires there's a nested <React.StrictMode /> inside of the hypothetical <React.NoStrict> component.

@johan-apo
Copy link

Take for example @uppy/react package. I am using NextJS now, and it would be great to disable Strict mode for only my component rendering that package and keeping the strictness everywhere else. According to NextJS documentation, we can use React.StrictMode on a page basis, but is a bit tedious because -for instance- from 15 pages, only one should not use StrictMode because of the package not working with it enabled. Correct me if I am wrong, please.

@jwhiting
Copy link

jwhiting commented Nov 1, 2022

What if a strict mode wrapper could have a prop that whitelists allowed subtree exceptions, which takes a list of identifiers that can be attached to a strict mode opt-out directive. So: <React.StrictMode allowedExceptions={["foo"]}><React.StrictModeOff id="foo">...</React.StrictModeOff><React.StrictMode/>

The use case here is to support the inclusion of third party library components that are not yet strict mode compatible without having to bend over backwards to keep strict mode enabled everywhere else. Yes, those third party libraries could have arbitrary children, and those children would not be safe, but they would be contained within a known boundary that third parties can't control - the top level app author needs to whitelist each use of opt-out.

To use the food on the plate analogy, we are saying "this plate is safe to eat except for the side of beans" rather than "this plate is safe to eat except where any arbitrary ingredient decided on its own to opt out."

@justin-calleja
Copy link

Hey sorry if I shouldn't be commenting on a closed issue. I was just wondering what advice anyone might have for this situation and don't know the proper channel to ask:

We have a BE endpoint that needs to be called once at a time to guarantee correctness (or whatever reason - but I can't change this). In other words, if I call this endpoint again while the first request is still processing, both requests fail.
What cleanup code can I make in useEffect's return function? To the best of my knowledge; I can't do anything. Aborting the request won't help, it's already made it to the BE and BE have already started the process. They (BE) are responsible for cleaning up in this case I guess but I'm out of luck because they don't care about my problems with React. I tried using some global state to prevent the call happening again.. but I need to clean this state up.. and the point that that happens is useEffect's return function which ... is useless as that would re-enable the call for happening when strict mode mounts the component again resulting in possible 2nd request making it to BE while first hasn't returned yet.

@L0rdCr1s
Copy link

@justin-calleja here is what I did to get around that

....
const [called, setCalled] = useState(false);

useEffect(() => {
    if (!called) {
       setCalled(true);
       //.... do your stuff here
   }
}, []);

...

That will ignore the second call during strict mode

@justin-calleja
Copy link

@L0rdCr1s thanks! I haven't tried this but it worked for you? Reason why I'm thinking it might not is because:

  • first mount, called is false and setCalled runs and API request is made.
  • component is unmounted and loses it's state (or am I wrong here?)
  • second mount, called is false again due to unmount and API request is made again.

But it's worth a shot if I have time to work on this.

Thanks again.

@JolivetteSax
Copy link

Warning spam from an old component I need, removed the strictmode tags from the entire app and moved on. Too bad there's no way to turn it off down around that one component and file away the technical debt for any future upgrades.

@Nantris
Copy link

Nantris commented Jan 28, 2023

Couldn't agree more with @JolivetteSax - it makes StrictMode all or nothing - and due to third-party dependencies - that means nothing.

@neiker
Copy link

neiker commented Jan 26, 2024

On react native we have LogBox.ignoreLogs(["warning to ignore"]);, I would LOVE to have the same for StrictMode. Right now we can't enable StrictMode because react-native-reanimated is still using findNodeHandle API and the terminal get flooded with:

ERROR Warning: findNodeHandle is deprecated in StrictMode. findNodeHandle was passed an instance of AnimatedComponent(Wrap) which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://react.dev/link/strict-mode-find-node

This would be easily overcome with something like: React.StrictMode.ignoreLogs(["findNodeHandle"])

@kelvinwop
Copy link

Wow classic case of dogma over practical usefulness. All we care about is getting error detection from Strict mode at a granular level, we don't care if you call it notstrictmode or whatever. But alright, you know everything, bigshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests