welldone-software / why-did-you-render

why-did-you-render by Welldone Software monkey patches React to notify you about potentially avoidable re-renders. (Works with React Native as well.)
https://www.npmjs.com/package/@welldone-software/why-did-you-render
MIT License
11.32k stars 199 forks source link

WDYR includes full lodash library with window._ override #274

Closed extempl closed 10 months ago

extempl commented 1 year ago

This creates a lot of issues, starting with packages size, and ending with conflicts with underscore window._ in big old projects.

Please use tree-shaking (babel-plugin-lodash) or separate packages (aka import deepClone from 'lodash.deepClone').

vzaidman commented 1 year ago

hey! the library is supposed to only be used in dev.

the code

if (process.env.NODE_ENV === 'development') {
  const whyDidYouRender = require('@welldone-software/why-did-you-render');
  // ...
}

makes sure that it doesn't affect the production code.

extempl commented 1 year ago

@vzaidman It still corrupts results on dev for me. Some parts of app does not work when window._ is overridden. Also, when it is switched off it is still ends up in the source code (and imports lodash) because of importSource: '@welldone-software/why-did-you-render', I guess.

extempl commented 1 year ago

@vzaidman Just checked - same for prod - WDYR itself does not called, but the code is there, and lodash is there, and override is there. With all process.env.NODE_ENV === 'development' checks.

vzaidman commented 1 year ago

Gotcha. I'll make sure there's no window._ override and revise the guidelines not to include the line importSource: '@welldone-software/why-did-you-render' in process.env.NODE_ENV === 'development'.

I'd also try to find a way to compile the library as an empty file with console.error or something if the app is included in prod. The library should not be included in prod in any capacity.

vzaidman commented 10 months ago

OK I finally got to look at it. This is a very annoying issue indeed.

  1. lodash does not seem to be present in my prod no matter what I do. Please re-confirm this if possible @extempl.

  2. In dev, the issue is essentially https://github.com/webpack/webpack/issues/4465. I couldn't reproduce it initially because I've been building with rollup and I assume you've been using webpack. The thing is that because my library doesn't build lodash or pack it in any way, but just imports it, the issue is between you, your building infra and lodash (the issue linked above). Now I know you didn't import lodash so it's a nuasance you didn't call for, but I can't do anything in this regard, it's a lodash<>webpack issue. If you want, call lodash.noConflict(); before or after importing WDYR like in the example below, but I don't want to call lodash.noConflict(); myself because it might introduce an unexpected behaviour for users relying on it.

    
    if (process.env.NODE_ENV === "development") {
    const lodash = require("lodash");
    lodash.noConflict();
    const whyDidYouRender = require("@welldone-software/why-did-you-render");
    whyDidYouRender(React, {
    trackAllPureComponents: true,
    });
    }
extempl commented 10 months ago

I ended up removing WDYR from the code, adding it manually locally only to check something, never committing to repo. But it would be easier to not do that of course.

I do not remember exactly, but I think what I meant by description here

Please use tree-shaking (babel-plugin-lodash) or separate packages (aka import deepClone from 'lodash.deepClone')

is that we actually use both of these approaches and it does not override window._, while WDYR (and only it) uses full import which does that override. So I still think this is the problem in how you do it in WDYR (Or, rather, how you could fix it. I doubt there are users of WDYR that are relying on WDYR's lodash in window._. And if there are… Well…).

I have a project where react co-lives with backbone (… hard to get rid of it), and all that with lazy loading and switching one with another, so what I did initially was that conditional noConflict and that was honestly hell. Until I found that WDYR import responsible for that and just removed it completely.

Perhaps that is webpack issue that it includes full lodash, if lodash is partially used in the project with tree-shaking, if some of packages includes full package. Some kind of optimisation? That's why you might not have it in your prod bundle.

So, I still believe that the easiest way would be to fix imports in WDYR.