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

class constructors must be invoked with |new| #5

Closed screaney closed 5 years ago

screaney commented 5 years ago

Trying to implement base functionality on our existing React app.

Getting

class constructors must be invoked with |new|

for any component I add the static property to.

It looks like this is a common issue across this library and the predecessors, but I was under the impression this library had it solved, so I must be missing something.

I've attached the library via NPM as well as manually adding the src to see if I could narrow it down. Best I can tell the error is thrown here (line 505): _this = _possibleConstructorReturn(this, _getPrototypeOf(WDYRPatchedClassComponent).call(this, props, context));

Excluding markup, my component looks like this: `class Home extends React.Component {

constructor(props) { super(props); } static whyDidYouRender = true; render(){ ... } export default Home; `

vzaidman commented 5 years ago
  1. do you use the latest 2.4.0 version? latest 16.7.0 React?
  2. can you try to debug the library at function "createPatchedComponent"? it has the code
    Component.prototype && Component.prototype.isReactComponent

    that is suppose to detect your component is a react component. what does it returns?

screaney commented 5 years ago

I modified that call to be like this:

function createPatchedComponent(componentsMap, Component, displayName, React, options) {
  let isClass = Component.prototype && Component.prototype.isReactComponent;
  if (isClass) {
    return patchClassComponent(Component, displayName, React, options);
  }

  return patchFunctionalComponent(Component, displayName, React, options);
}

When I debug it, isClass is {} (an empty object).

vzaidman commented 5 years ago

this is what suppose to be there hmmmmm

vzaidman commented 5 years ago

try

class Home extends React.Component {
  render(){
    ...
  }
}

Home.whyDidYouRender = true

export default Home;
screaney commented 5 years ago

Same Result

vzaidman commented 5 years ago

do you use https://babeljs.io/docs/en/next/babel-plugin-proposal-class-properties.html ?

screaney commented 5 years ago

Yes, our webpack is

"plugins": [
                        "@babel/plugin-syntax-dynamic-import",
                        "@babel/plugin-syntax-import-meta",
                        ["@babel/plugin-proposal-class-properties", {
                            "loose": false
                        }],
                        "@babel/plugin-proposal-json-strings"
                    ]
vzaidman commented 5 years ago

react 16.7.0?

screaney commented 5 years ago

I could've sworn it was 16.7, but it looks like it's 16.6, updating now, will retest

vzaidman commented 5 years ago

plesae try

import whyDidYouRender from '@welldone-software/why-did-you-render/dist/umd/whyDidYouRender';
screaney commented 5 years ago

Updated to React 16.7, also changed to using that import. No change, same error.

vzaidman commented 5 years ago

it might be related to babel-polyfill. do you have a polyfill?

vzaidman commented 5 years ago

also, by reading https://github.com/babel/babel/issues/7022 it might be the class being extended is transpiled differently then how it is extended in the library.

vzaidman commented 5 years ago

ok, please try yarn add git+https://github.com/welldone-software/why-did-you-render.git#dont-transform-classes i think i identified the problem

andrew1007 commented 5 years ago

Fixed it for me :). Much thanks

vzaidman commented 5 years ago

i belive what we see is a transpiled by babel class trying to extend a not transpiled class. so we have several options- make the users transpile their classes / dont transpile classes in the library both options seem bad for obvious reasons. i'll see what i can do.

asked about this in reddit: https://www.reddit.com/r/javascript/comments/agvihk/how_to_deal_with_class_extension_in_libraries/

screaney commented 5 years ago

I pulled your branch as recommended and it now appears to be working.

vzaidman commented 5 years ago

fixed in v2.5.0.

If you are not transpiling your classes, the library should be built without class transpilations as well.

So we added 3 endpoints where it is indeed not transpiled:

  "main-no-classes-transpile": "dist/no-classes-transpile/cjs/whyDidYouRender.min.js",
  "module-no-classes-transpile": "dist/no-classes-transpile/esm/whyDidYouRender.min.js",
  "browser-no-classes-transpile": "dist/no-classes-transpile/umd/whyDidYouRender.min.js",

to use it, import the library like this:

const whyDidYouRender = require('@welldone-software/why-did-you-render/dist/no-classes-transpile/umd/whyDidYouRender.min.js');
vzaidman commented 5 years ago

@screaney does it works for you?

also notice we can eliminate these endpoints once https://github.com/babel/babel/pull/8656 is merged.

mschipperheyn commented 5 years ago

Finally was able to create a workaround. I'm extending an existing babel config that covers both client and server and I'm running into this on the node side of things. But the key is to exclude babel-plugin-transform-classes / @babel/plugin-transform-classes

const razzleBabel = require('razzle/babel');

module.exports = function babelConfig(api) {
    api.cache(true);

    const babel = razzleBabel();

    babel.presets[0] =
        process.env.BUILD_TARGET === 'client'
            ? [
                require.resolve('@babel/preset-env'),
                {
                    modules: false,
                },
              ]
            : [
                require.resolve('@babel/preset-env'),
                {
                    targets: {
                        node: 'current',
                    },
                    exclude: [
                        'babel-plugin-transform-classes',
                        '@babel/plugin-transform-classes',
                    ],
                    modules: false,
                },
              ];
                  [..]
vzaidman commented 5 years ago

again:

frederikhors commented 5 years ago

Same error here using the create-react-app 3 and this import:

const whyDidYouRender = require('@welldone-software/why-did-you-render/dist/no-classes-transpile/umd/whyDidYouRender.min.js').

My browserlist:

  "browserslist": {
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  }

Maybe you should re-open this issue.

maxportmc commented 5 years ago

Getting the same issue as @frederikhors using create-react-app. Is there any workaround?

vzaidman commented 5 years ago

I don't see this bug in the sandbox: https://codesandbox.io/s/welldone-softwarewhy-did-you-render-with-reactredux-fc8es

try to reproduce it please.

vzaidman commented 5 years ago

I think i get it. You are using "last 1 safari version" so you DO need to use the default version of the library:

const whyDidYouRender = require("@welldone-software/why-did-you-render");

The no-classes-transpile version is only if you build for latest browsers where the class keyword is supported natively.

the last 1 safari version line in browserslist makes it so Babel does transpile your classes.

@frederikhors @maxportmc

maxportmc commented 5 years ago

I have given it a go with the default version but no luck.I get the same error

"TypeError: Class constructor Dashboard cannot be invoked without 'new'"

After some messing around I got it working in some components and realised it is failing in all the components with react-redux's connect import.

vzaidman commented 5 years ago

are you using the last react-redux version?

maxportmc commented 5 years ago

Was on version 5.1.0, just updated to 7.1.0 but same error using both versions of the library. I will see if I can recreate the error in a sandbox.

vzaidman commented 5 years ago

I decided to close this issue since I understand very well why it happens and that it is solved already: https://github.com/welldone-software/why-did-you-render/issues/5#issuecomment-467900253

If anybody has new information and / or able to reproduce it here: https://codesandbox.io/s/welldone-softwarewhy-did-you-render-with-reactredux-fc8es

I'll obviously re-open it.