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

class constructors must be invoked with |new| #5

Closed
screaney opened this issue Jan 15, 2019 · 29 comments
Closed

class constructors must be invoked with |new| #5

screaney opened this issue Jan 15, 2019 · 29 comments
Labels
bug Something isn't working

Comments

@screaney
Copy link

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
Copy link
Collaborator

  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
Copy link
Author

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
Copy link
Collaborator

this is what suppose to be there hmmmmm

@vzaidman
Copy link
Collaborator

vzaidman commented Jan 16, 2019

try

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

Home.whyDidYouRender = true

export default Home;

@screaney
Copy link
Author

Same Result

@vzaidman
Copy link
Collaborator

@screaney
Copy link
Author

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
Copy link
Collaborator

react 16.7.0?

@screaney
Copy link
Author

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

@vzaidman
Copy link
Collaborator

plesae try

import whyDidYouRender from '@welldone-software/why-did-you-render/dist/umd/whyDidYouRender';

@screaney
Copy link
Author

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

@vzaidman
Copy link
Collaborator

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

@vzaidman
Copy link
Collaborator

also, by reading babel/babel#7022
it might be the class being extended is transpiled differently then how it is extended in the library.

@vzaidman
Copy link
Collaborator

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
Copy link

Fixed it for me :). Much thanks

@vzaidman
Copy link
Collaborator

vzaidman commented Jan 17, 2019

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
Copy link
Author

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

@vzaidman
Copy link
Collaborator

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
Copy link
Collaborator

@screaney does it works for you?

also notice we can eliminate these endpoints once babel/babel#8656 is merged.

@mschipperheyn
Copy link

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
Copy link
Collaborator

vzaidman commented Feb 27, 2019

again:

const whyDidYouRender = require('@welldone-software/why-did-you-render);
  • If you DO use native es6 classes, use:
const whyDidYouRender = require('@welldone-software/why-did-you-render/dist/no-classes-transpile/umd/whyDidYouRender.min.js');

another way to fix it is to enable @babel/plugin-transform-classes in @babel/preset-env like this:

presets: [
      ['@babel/preset-env', {
        //...
        include: [
          // remove when `https://github.com/babel/babel/issues/7022` is solved
          '@babel/plugin-transform-classes'
        ]
      }],
      '@babel/preset-react',
      //.....
    ],
//.....

@frederikhors
Copy link

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
Copy link

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

@vzaidman
Copy link
Collaborator

vzaidman commented Jul 18, 2019

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 vzaidman reopened this Jul 18, 2019
@vzaidman vzaidman added cant reproduce Elaboration on how to reproduce the issue is needed bug Something isn't working labels Jul 18, 2019
@vzaidman
Copy link
Collaborator

vzaidman commented Jul 18, 2019

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

@vzaidman vzaidman removed the cant reproduce Elaboration on how to reproduce the issue is needed label Jul 18, 2019
@maxportmc
Copy link

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
Copy link
Collaborator

are you using the last react-redux version?

@maxportmc
Copy link

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
Copy link
Collaborator

vzaidman commented Sep 9, 2019

I decided to close this issue since I understand very well why it happens and that it is solved already:
#5 (comment)

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.

@vzaidman vzaidman closed this as completed Sep 9, 2019
vzaidman added a commit that referenced this issue Aug 15, 2020
* make logOwnerReasons true by default.
* only trackHooks if they are supported.
* removed building without .babel-plugin-transform-classes because babel fixed it in their newer versions. removed a mention to this from readme. this closes #5 closes #131.
* removed babel-plugin-lodash - we don't really care about the bundle size and also the user of the library might use their own lodash optimizations instead.
* we only build a "umd" version now. Since the package is only for development, we don't really care about different build types.
* improved readme in general, including the installation tips. this closes #130.
vzaidman added a commit that referenced this issue Oct 22, 2020
* make logOwnerReasons true by default.
* only trackHooks if they are supported.
* removed building without .babel-plugin-transform-classes because babel fixed it in their newer versions. removed a mention to this from readme. this closes #5 closes #131.
* removed babel-plugin-lodash - we don't really care about the bundle size and also the user of the library might use their own lodash optimizations instead.
* we only build a "umd" version now. Since the package is only for development, we don't really care about different build types.
* improved readme in general, including the installation tips. this closes #130.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants