docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard
verified publisher icon facebook

Discrepancy between dev and prod build behavior (TypeError: class constructors must be invoked with 'new')

Open devalbo opened this issue 3 years ago • 4 comments

Have you read the Contributing Guidelines on issues?

Prerequisites

  • [X] I'm using the latest version of Docusaurus.
  • [X] I have tried the npm run clear or yarn clear command.
  • [X] I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • [X] I have tried creating a repro with https://new.docusaurus.io.
  • [X] I have read the console error message carefully (if applicable).

Description

I created a repo to demonstrate the issue here: https://github.com/devalbo/docusaurus-protobuf-test

I want to use the package @protobuf-ts/plugin to use some data in a docusaurus page. Everything was going great in dev mode. However, once I did a local build with npm run build/npm run serve, when I go to a test page, I get an error similar to the stack trace below. I have 2 test pages - one with the protobuf directly in the function call and one where it's in a called function. I see the error in both of these pages in the prod build.

I'm also not seeing this behavior if I do a fresh create-react-app with Typescript support and then run npm run build/serve -s build.

I figure there's something going on in the build process I'm not aware of (or able to figure out :( ) that's causing this. Is there some type of docusaurus thing I need to configure for this to work the same way in both dev and prod?

Uncaught (in promise) TypeError: class constructors must be invoked with 'new'
    a http://localhost:3000/assets/js/46169e7c.eaa30263.js:1
    5568 http://localhost:3000/assets/js/46169e7c.eaa30263.js:1
    o http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    promise callback*723/u["46169e7c"]< http://localhost:3000/assets/js/main.45756f3d.js:2
    d http://localhost:3000/assets/js/main.45756f3d.js:2
    f http://localhost:3000/assets/js/main.45756f3d.js:2
    f http://localhost:3000/assets/js/main.45756f3d.js:2
    h http://localhost:3000/assets/js/main.45756f3d.js:2
    preload http://localhost:3000/assets/js/main.45756f3d.js:2
    N http://localhost:3000/assets/js/main.45756f3d.js:2
    N http://localhost:3000/assets/js/main.45756f3d.js:2
    9383 http://localhost:3000/assets/js/main.45756f3d.js:2
    o http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
    O http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
    t http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
46169e7c.eaa30263.js:1:238

Reproducible demo

https://github.com/devalbo/docusaurus-protobuf-test

Steps to reproduce

See steps in repo README.

Expected behavior

I would expect the same behavior from dev and prod builds when I use the test pages (ideally, one where I can use the protobuf-ts plugin), but it really sucks to not have the same results in prod as I get in dev.

Actual behavior

I'm also not seeing this behavior if I do a fresh create-react-app with Typescript support and then run npm run build/serve -s build.

Stack trace below happens only on prod build when I go to test pages:

Uncaught (in promise) TypeError: class constructors must be invoked with 'new'
    a http://localhost:3000/assets/js/46169e7c.eaa30263.js:1
    5568 http://localhost:3000/assets/js/46169e7c.eaa30263.js:1
    o http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    promise callback*723/u["46169e7c"]< http://localhost:3000/assets/js/main.45756f3d.js:2
    d http://localhost:3000/assets/js/main.45756f3d.js:2
    f http://localhost:3000/assets/js/main.45756f3d.js:2
    f http://localhost:3000/assets/js/main.45756f3d.js:2
    h http://localhost:3000/assets/js/main.45756f3d.js:2
    preload http://localhost:3000/assets/js/main.45756f3d.js:2
    N http://localhost:3000/assets/js/main.45756f3d.js:2
    N http://localhost:3000/assets/js/main.45756f3d.js:2
    9383 http://localhost:3000/assets/js/main.45756f3d.js:2
    o http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
    O http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
    t http://localhost:3000/assets/js/runtime~main.9bc87d77.js:1
    <anonymous> http://localhost:3000/assets/js/main.45756f3d.js:2
46169e7c.eaa30263.js:1:238

Your environment

  • Public source code: https://github.com/devalbo/docusaurus-protobuf-test
  • Public site URL:
  • Docusaurus version used: 2.0.0-beta.21
  • Environment name and version (e.g. Chrome 89, Node.js 16.4): npm 8.5.0, node v16.14.2
  • Operating system and version (e.g. Ubuntu 20.04.2 LTS): mac OS Montery (12.4)

Self-service

  • [ ] I'd be willing to fix this bug myself.

devalbo avatar Jun 12 '22 20:06 devalbo

Dev and prod have different Babel settings (I think) and different minification levels, so it's totally possible that this happens. However we do compile with loose: true, which theoretically should enable the noClassCalls assumption...

Josh-Cena avatar Jun 13 '22 02:06 Josh-Cena

Random debug notes

The offending line is

export const Person = new Person$Type();

Cannot attach debugger in prod mode so this is a pain to debug.

Hacked into @docusaurus/core/lib/babel/preset.js and turned off loose. The page now errors with

main.5964daac.js:2 Uncaught ReferenceError: exports is not defined

This seems unrelated and I don't have time to debug this, so turned loose: true on again. This time was able to narrow it down to one line of minified code:

r=function(n){function o(){return n.call(this,"Person",[{no:1,name:"name",kind:"scalar",T:9},{no:2,name:"id",kind:"scalar",T:4,L:0},{no:3,name:"years",kind:"scalar",jsonName:"baz",T:5},{no:5,name:"data",kind:"scalar",opt:!0,T:12}])||this}return(0,a.Z)(o,n),o}(e(5102).C)

This line corresponds to the super("Person", [...]) line. The n.call(...) is calling e(5102).C as a function. e(5102).C is a chunk that corresponds to import { MessageType } from "@protobuf-ts/runtime";. I think it's because we use loose which also enables superIsCallableConstructor. Otherwise, it should be using Reflect.construct.

If @protobuf-ts/runtime is part of the site, it will be transpiled by Babel and therefore will be compiled to a callable function. However, node_modules aren't transpiled, and MessageType stays as a class. This means a transpiled class, which assumes its super class to be transpiled as well, actually ending up super'ing an un-transpiled class. To fix this, I added @protobuf-ts/runtime to this list:

https://github.com/facebook/docusaurus/blob/27834dc23a194e6815140822c6b1b381a52dffc3/packages/docusaurus/src/webpack/base.ts#L26-L28

And it works properly now:

image

There are three ways to fix this:

  1. Expose the LibrariesToTranspile setting. I personally find it quite useful.

  2. We should be more opinionated about what kinds of assumptions we make. For example, the iterableIsArray assumption has caused bugs with [...new Set()] in the past. https://github.com/facebook/docusaurus/issues/4972#issuecomment-863895061 Maybe we can only turn on the assumptions that seem safe to make (noDocumentAll, noClassCalls, setClassMethods ...) while disabling other assumptions (can be re-enabled in userland if desired)

  3. Userland solution: in babel.config.js, add this:

    module.exports = {
  presets: [require.resolve('@docusaurus/core/lib/babel/preset')],
  assumptions: {
    superIsCallableConstructor: false,
  }
};

    And I can confirm this works. cc @devalbo before we "fix" this on our end (if at all), you should compile with this config.

Back to the question of why it isn't showing up in dev mode. It's because the dev mode's browserslist config is very strict and only includes recent versions of evergreen browsers, so all classes aren't transpiled. If you set your development browserslist config to the same as production, you will get the same error in development now.

Josh-Cena avatar Jun 13 '22 03:06 Josh-Cena

I can confirm updating babel.config.js as described in Fix 3 works for me. Thank you!

devalbo avatar Jun 13 '22 12:06 devalbo

Hello, this may be off topic but I saw your point 2. @Josh-Cena

We have several [...new Set()] in our code base but they are not transformed well by babel, I try to disable iterableIsArray in my babel.config.js but it breaks others stuff (build is done but Uncaught ReferenceError: exports is not defined in browser console)

Is there a way to fix this just by a configuration change ? Or I need to change all [...new Set()] to Array.from(new Set()) as in https://github.com/facebook/docusaurus/pull/5000 ?

wiinxt avatar Nov 29 '22 11:11 wiinxt