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.24k stars 201 forks source link

trackExtraHooks cannot set property of #<Object> which has only a getter #85

Open mwskwong opened 4 years ago

mwskwong commented 4 years ago

By specifying trackExtraHooks, the following error is thrown. NOTE: The error is only thrown when running in local machine, NOT in codesandbox.

Reproduction link: Edit why-did-you-render-test

whyDidYouRender.js:167 Uncaught TypeError: Cannot set property useSelector of #<Object> which has only a getter
    at whyDidYouRender.js:167
    at Array.forEach (<anonymous>)
    at zn (whyDidYouRender.js:150)
    at Module../src/index.js (index.js:9)
    at __webpack_require__ (bootstrap:785)
    at fn (bootstrap:150)
    at Object.1 (styles.css?f684:43)
    at __webpack_require__ (bootstrap:785)
    at checkDeferredModules (bootstrap:45)
    at Array.webpackJsonpCallback [as push] (bootstrap:32)
    at main.chunk.js:1
(anonymous) @ whyDidYouRender.js:167
zn @ whyDidYouRender.js:150
./src/index.js @ index.js:9
__webpack_require__ @ bootstrap:785
fn @ bootstrap:150
1 @ styles.css?f684:43
__webpack_require__ @ bootstrap:785
checkDeferredModules @ bootstrap:45
webpackJsonpCallback @ bootstrap:32
(anonymous) @ main.chunk.js:1
manifest.json:1 Manifest: Line: 1, column: 1, Syntax error.
vzaidman commented 4 years ago

try using import * as ReactRedux from 'react-redux' instead.

mwskwong commented 4 years ago

try using import * as ReactRedux from 'react-redux' instead.

Codesandbox edited Same result

vzaidman commented 4 years ago

I think it has to do with your transpiler (probably babel). put a breakpoint before running WDYR.

image

see if React is there, WDYR is there and also there's a "useSelector" in React Redux

mwskwong commented 4 years ago

It seems that React, WDYR and useSelector all exists. It's just the fact that the compiler thinks WDYR is trying to set a getter-only property.

image image

vzaidman commented 4 years ago

what version of webpack do you use?

mwskwong commented 4 years ago

I'm using CRA so it is managed internally.

vzaidman commented 4 years ago

same version as in the sandbox?

On Fri, Jan 31, 2020 at 11:09 AM matthewkwong2 notifications@github.com wrote:

I'm using CRA, and does NOT eject so it is managed internally.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/welldone-software/why-did-you-render/issues/85?email_source=notifications&email_token=ABHSW25Z3T3J4WLJ7HYHQL3RAPTCZA5CNFSM4KOCOXJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKN765A#issuecomment-580648820, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHSW2YEJRMYXHANE2Q46LTRAPTCZANCNFSM4KOCOXJQ .

mwskwong commented 4 years ago

All latest

mwskwong commented 4 years ago
   "@welldone-software/why-did-you-render": "latest",
    "react": "latest",
    "react-dom": "latest",
    "react-redux": "latest",
    "react-scripts": "latest",
    "redux": "latest"
vzaidman commented 4 years ago

Sadly, webpack produces immutable objects when compiles es imports and exports. Currently, the only way to make it work is to use the "umd" version of the library being exported. At least in development:

    resolve: {
      alias: {
        'react-redux': process.env.NODE_ENV === 'development' ? 'react-redux/lib' : 'react-redux'
      }
    }
DalderupMaurice commented 4 years ago

How are we able to fix this without ejecting the webpack config? I am unable to use the library as intended with this issue and IMO it shouldn't be low priority/wontfix at all as a lot of people use CRA and will probably experience the same issue.

vzaidman commented 4 years ago

I believe theres no way to do it with CRA alone at the moment.

You can use: https://www.npmjs.com/package/react-app-rewired

To add alias to webpack as mentioned above.

karol-majewski commented 4 years ago

In my Webpack config, I included the UMD build by doing:

    alias: {
      'react-redux': isDevelopment ? 'react-redux/dist/react-redux.js' : 'react-redux/lib',
    },

The Cannot set property useSelector of #<Object> which has only a getter error is gone. However, how do I know it actually worked? I created a component based on the example to see if the silly usage of useSelector gets detected.

const User: React.FunctionComponent = React.memo(() => {
  const s = useSelector(state => ({ auth: { userId: state.auth?.userId } }));

  return <div>{s.auth?.userId}</div>;
});

My settings:

  whyDidYouRender(React, {
    trackExtraHooks: [[require('react-redux'), 'useSelector']],
  });

Nothing ever gets logged in the console. What am I doing wrong?

vzaidman commented 4 years ago

the library only tracks the re-renders (including re-renders as a result of hook changes) on selected components.

you are missing the tracking of the "User" component.

to do so, add:

User.whyDidYouRender = true;

or you can set the library to track all pure components:

  whyDidYouRender(React, {
    trackAllPureComponents: true,
    trackExtraHooks: [[require('react-redux'), 'useSelector']],
  });
karol-majewski commented 4 years ago

My apologies. I did have whyDidYouRender enabled, but my mistake was even more silly. I was using a proxied selector that wraps the one provided by react-redux.

import { useSelector as useSelector_ } from 'react-redux';

export const useSelector: <T>(selector: (state: State) => T, equalityFn?: (left: T, right: T) => boolean) => T = useSelector_;

The purpose of doing that is having your State defined every time you call useSelector. It calls the same function, only the reference is different, so it was not picked up by why-did-you-render. I imagine this pattern will become more widespread as Hooks get more popular.

I imagine the solution for that is the same as before — importing a mutable object instead of (immutable) ECMAScript module. However, this could evolve badly if more and more modules receive special treatment from Webpack configuration.

Do you think it would be possible to have an API that accepts a reference to the augmented hook?

import * as hooks from "whatever";

whyDidYouRender(React, {
    trackAllPureComponents: true,
    trackExtraHooks: [hooks.useSelector],
  });

ECMAScript module objects are immutable, but their constituent members are not. This could allow us to have the best of two worlds — run whyDidYouRender on custom hooks without having to alter the Webpack configuration file.

vzaidman commented 4 years ago

it's not possible as far as i know. what you can try to do is to export useSelector in a default export:

const exports = {
  useSelector
}

export default exports 
nour-s commented 4 years ago

I'm using CRA, and I managed to get around it by doing:

if (process.env.NODE_ENV === 'development') { whyDidYouRender(React, { trackAllPureComponents: true, trackHooks: true, trackExtraHooks: [[require('react-redux/lib'), 'useSelector']], }); }

vzaidman commented 4 years ago

but then you probably need to use import {useSelector} from 'react-redux/lib' anywhere so useSelector would be tracked, no?

nour-s commented 4 years ago

Interesting, actually I didn't change any of that, not sure how it worked then !

vzaidman commented 4 years ago

Do you see why did you render console warnings about useSelector returning an object which equals by value and different by reference?

did you try:

const something = useSelector(state => ({a: state.a}))
vzaidman commented 4 years ago

here is an example of how it works in practise: https://codesandbox.io/s/welldone-softwarewhy-did-you-render-with-reactredux-fc8es

notice when clicking on "same value" how there's a "why did you render" log in sandbox's console.

now, the sandbox uses react-redux/lib for some reason, but im not sure what's going on under the hood there...

armspkt commented 4 years ago

@vzaidman

trackExtraHooks: [[require('react-redux/lib'), 'useSelector']],

it's work. thanks.

feimosi commented 4 years ago

I can confirm that only setting the package alias to react-redux/dist/react-redux.js fixes the issue.

trackExtraHooks: [[require('react-redux/lib'), 'useSelector']] doesn't work, it only silences the error but you won't get updates from useSelector.

For people using rescripts here's a copy&paste solution:

[
  'use-rewire',
  (config, NODE_ENV) => {
    config.resolve.alias = {
      ...config.resolve.alias,
      'react-redux': NODE_ENV === 'development' ? 
        'react-redux/dist/react-redux.js' : 'react-redux/lib',
    };

    return config;
  }
]
vzaidman commented 4 years ago

trackExtraHooks: [[require('react-redux/lib'), 'useSelector']] doesn't work, it only silences the error but you won't get updates from useSelector.

yes, because later imports won't use react-redux/lib but react-redux/dist so running this code by itself won't help.

You also need to ensure later imports use react-redux/lib.

chaomao commented 4 years ago

Hi, @vzaidman I am using nextjs, react-redux and awesome why-did-you-render, I wonder my following changes will enable why-did-you-render to monitor useSelector? in next.config.js

        webpack: (config) => {
          config.resolve.alias = {
            ...config.resolve.alias,
            'react-redux': process.env.NODE_ENV === 'development' ?
              'react-redux/dist/react-redux.js' : 'react-redux/lib',
          };
          return config;

in _app.js

if (process.env.NODE_ENV !== "production") {
  const whyDidYouRender = require("@welldone-software/why-did-you-render");

  whyDidYouRender(React, {
    trackAllPureComponents: true,
    trackHooks: true,
    trackExtraHooks: [[require('react-redux'), 'useSelector']],
  });
}

in component.js

import { useDispatch, useSelector } from "react-redux";
const SomeComponent = () =>{
const currentUser = useSelector(state => state.currentUser);
}

SomeComponent.WhyDidYouRender = true
export default SomeComponent;

I don't have any luck to monitor useSelector changes? any hint to me? thanks

vzaidman commented 4 years ago

Damn I messed up in my comment on how to deal with it and reversed the if statement. I'll rewrite the comment from scratch again:

Sadly, webpack produces immutable objects when compiles es imports and exports. Currently, the only way to make it work is to use the "umd" version of the library being exported at least in development mode:

resolve: {
  alias: {
    'react-redux': process.env.NODE_ENV === 'development' ? 'react-redux/lib' : 'react-redux'
  }
}
psychedelicious commented 4 years ago

This works in CRA without anything else. Edit node_modules/react-scripts/config/webpack.config.js, around line 300 there is the alias section:

alias: {
  // Support React Native Web
  // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/
  'react-native': 'react-native-web',
  // Allows for better profiling with ReactDevTools
  ...(isEnvProductionProfile && {
    'react-dom$': 'react-dom/profiling',
    'scheduler/tracing': 'scheduler/tracing-profiling',
  }),
  // WHY DID YOU RENDER:
  'react-redux': process.env.NODE_ENV === 'development' ? 'react-redux/lib' : 'react-redux',
  ...(modules.webpackAliases || {}),

I am getting notifications in console per examples and seeing why things are rendered.

vzaidman commented 4 years ago

This works in CRA without anything else. Edit node_modules/react-scripts/config/webpack.config.js, around line 300 there is the alias section:

alias: {
  // Support React Native Web
  // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/
  'react-native': 'react-native-web',
  // Allows for better profiling with ReactDevTools
  ...(isEnvProductionProfile && {
    'react-dom$': 'react-dom/profiling',
    'scheduler/tracing': 'scheduler/tracing-profiling',
  }),
  // WHY DID YOU RENDER:
  'react-redux': process.env.NODE_ENV === 'development' ? 'react-redux/lib' : 'react-redux',
  ...(modules.webpackAliases || {}),

I am getting notifications in console per examples and seeing why things are rendered.

It would indeed work, but I would not suggest editing anything in node_modules. Better use one of the libraries that help you to patch CRA

psychedelicious commented 4 years ago

Yes, I should have elaborated.

I used the patch-package:

node_modules is untouched and we have a simple way to revert the change in the future:

mwskwong commented 4 years ago

We have a workaround for react-redux, but what about other popular libraries like react-router-dom and @material-ui/core? This workaround will only work for react-redux.

vzaidman commented 4 years ago

what do you want to track in react-router-dom?

regarding @material-ui/core, they also have a cjs build. look at node_modules/@material-ui/core for the cjs version and do exactly what we did with react-redux

steveadams commented 4 years ago

Not sure if this helps anyone, but I made a Gatsby plugin which accomplishes this. It seems to work well for me, and the solutions here might be transferable to another project.

https://www.npmjs.com/package/gatsby-plugin-why-did-you-render-redux

thoughtsofcode commented 4 years ago

I'm using Parcel instead of webpack and the above workaround doesn't seem to work.

Adding the alias "react-redux": "react-redux/lib" results in this build error:

Cannot resolve dependency 'react-redux'

on the line:

const ReactRedux = require("react-redux");

Is there another way to make this work with Parcel?

steveadams commented 4 years ago

@thoughtsofcode I hate to ask dumb questions, but did you ensure that react-redux is in your dependencies and currently installed in the node_modules directory?

thoughtsofcode commented 4 years ago

Yes, react-redux is installed. I'm able to build and run my app, which uses the useSelector hook, if I just comment out the line that imports wdyr.js.

vzaidman commented 4 years ago

can you check your node_modules for the existence of react-redux/lib ?

vzaidman commented 4 years ago

also it worth trying specifying an absolute route to the library something like:

./node_modules/react-redux/lib/index.js
thoughtsofcode commented 4 years ago

also it worth trying specifying an absolute route to the library something like:

./node_modules/react-redux/lib/index.js

Thanks! Changing the alias to this path resolved the build error.

Xadoy commented 4 years ago
  • yarn patch-patch react-scripts

Yes, I should have elaborated.

I used the patch-package:

  • Install as dev dependency: yarn add patch-package postinstall-postinstall -D
  • Edit webpack.config.js as above, inserting 'react-redux': process.env.NODE_ENV === 'development' ? 'react-redux/lib' : 'react-redux', in the alias: section just above ...(modules.webpackAliases || {}),, which is at line 309 for me - don't forget the comma!
  • Create patch: yarn patch-patch react-scripts - makes a patch which is created in automatically created directorypatches/
  • Add a new script to the CRA package.json: "prepare": "patch-package"

node_modules is untouched and we have a simple way to revert the change in the future:

  • Un-patch the file: yarn patch-package --reverse (will fail if the file has changed since it was patched, in this case can just do yarn add react-scripts --check-files to revert it)
  • Delete the patch file
  • Edit package.json, removing the added script
  • Remove the dev dependencies if no longer needed: yarn remove patch-package postinstall-postinstall

I believe you meant to say yarn patch-package react-scripts not patch-patch

Nantris commented 4 years ago

I thought the workaround was simple enough, but unfortunately it doesn't work for us. It makes at least one of our dependencies throw errors about not being able to find the Redux store and how we need to wrap our app in <Provider> from react-redux. We used the Webpack resolve method described above.

I did eventually get it working after some Webpack configuration changes. Unfortunately I'm not sure what change made the difference.

vzaidman commented 4 years ago

More info is required to help with it.

AdrienLemaire commented 4 years ago

We have a workaround for react-redux, but what about other popular libraries like react-router-dom and @material-ui/core? This workaround will only work for react-redux.

It doesn't seem possible to track react-router hooks on their dev & dev-experimental branches. I created an issue: https://github.com/ReactTraining/react-router/issues/7646

AdrienLemaire commented 4 years ago

So I got a reply from @timdorr who basically said that it should work with [require("react-router-dom/main"), "useLocation"].

The main.js checks the NODE_ENV and imports the correct commonjs file from their umd/ directory.

But it still doesn't work.

import React from "react";

if (process.env.NODE_ENV === "development") {
  const whyDidYouRender = require("@welldone-software/why-did-you-render");
  whyDidYouRender(React, {
    trackAllPureComponents: true, // useState, useEffect ...
    trackExtraHooks: [
      [require("@apollo/client/react/hooks/hooks.cjs"), "useReactiveVar"],
      [require("react-i18next/dist/commonjs/useTranslation"), "useTranslation"],
      [require("react-router-dom/main"), "useLocation"], // <= Cannot set property useLocation of #<Object> which has only a getter
  });

When I try to debug that, I can confirm that require("react-router-dom/main").useLocation exists.

if (process.env.NODE_ENV === "development") {
  const reactRouterDom = require("react-router-dom/main");
  console.log(reactRouterDom.useLocation);
  const whyDidYouRender = require("@welldone-software/why-did-you-render");
  whyDidYouRender(React, {
    trackExtraHooks: [
      [reactRouterDom, "useLocation"],
    ],
  });

@vzaidman any idea how to solve this ?

vzaidman commented 4 years ago

maybe react-router-dom/cjs.

but i wouldn't track "useLocation".

it sounds like a waste of time to me to be honest.

also, the way you import it here, should be the same way you import it everywhere in the project, so make sure to use "alias" or something.

DaleyKD commented 3 years ago

I tried doing this with msteams-react-base-component (v3.0.0), which has a cjs/esm directory. I had no luck and had to move on, which is a bummer because something is causing a 6x render.

mastrzyz commented 3 years ago

Reproduces for us when we tried to upgrade from Webpack4 to Webpack5 while on TS 4.1 and ES6.

jmtimko5 commented 3 years ago

Has anybody gotten this to work with @apollo/react-hooks? Attempted the approach here https://github.com/welldone-software/why-did-you-render/issues/85#issuecomment-612620664 and all the variations of importing with no luck.

nikitaNaredi commented 3 years ago

I can confirm that only setting the package alias to react-redux/dist/react-redux.js fixes the issue.

trackExtraHooks: [[require('react-redux/lib'), 'useSelector']] doesn't work, it only silences the error but you won't get updates from useSelector.

For people using rescripts here's a copy&paste solution:

[
  'use-rewire',
  (config, NODE_ENV) => {
    config.resolve.alias = {
      ...config.resolve.alias,
      'react-redux': NODE_ENV === 'development' ? 
        'react-redux/dist/react-redux.js' : 'react-redux/lib',
    };

    return config;
  }
]

Hey am using CRA, where I should add this react-redux/dist/react-redux.js. Can you specify the folder path and where and how exactly this needs to be added.

wdyr.js: if (process.env.NODE_ENV === "development") { const whyDidYouRender = require("@welldone-software/why-did-you-render"); whyDidYouRender(React, { trackAllPureComponents: true, trackHooks: true, trackExtraHooks: [[require("react-redux/lib"), "useSelector"]], }); }

It will be very kindfull if you guide me.

vzaidman commented 3 years ago

It will be very kindfull if you guide me.

please look at this comment: https://github.com/welldone-software/why-did-you-render/issues/154#issuecomment-773905769

nikitaNaredi commented 3 years ago

It will be very kindfull if you guide me.

please look at this comment: #154 (comment)

I have already followed this #154 But I need to track useSelector. For that I tried if (process.env.NODE_ENV === "development") { const whyDidYouRender = require("@welldone-software/why-did-you-render"); const ReactRedux = require("react-redux"); whyDidYouRender(React, { trackAllPureComponents: true, trackExtraHooks: [[ReactRedux, "useSelector"]], }); } But this gave me trackExtraHooks cannot set property of #<Object> which has only a getter

Then I follwed this issue and updated wdyr.js according to above mentioned solutions: if (process.env.NODE_ENV === "development") { const whyDidYouRender = require("@welldone-software/why-did-you-render"); whyDidYouRender(React, { trackAllPureComponents: true, trackHooks: true, trackExtraHooks: [[require("react-redux/lib"), "useSelector"]], }); } This also didn't give any logs.

Then I read this https://github.com/welldone-software/why-did-you-render/issues/85#issuecomment-605322912 And asking where and to add this react-redux/dist/react-redux.js. My react versions are: "react": "^17.0.1", "react-dom": "^17.0.1", "react-redux": "^7.2.2", "react-router-dom": "^5.2.0", "react-scripts": "4.0.0",

If you have any other solutions let me know. Am struggling around it from 4-5 days. Thanks