Parcel: Scope hoisting: wrapped non-toplevel var declarations

[1saf]

🐛 bug report

Creating a Parcel build for a React app in development and production, expecting the React application to load properly when it is served statically.

When running the parcel build with experimental scope hoisting and serving, it is runtime erroring out on different conditions.

🎛 Configuration (.babelrc, package.json, cli command)

.babelrc

{
    "plugins": [
        ["@babel/plugin-transform-runtime", {
            "regenerator": true
        }],
        "transform-es2015-arrow-functions",
        "@babel/proposal-class-properties"
    ],
    "presets": ["@babel/preset-env", "@babel/preset-react", "@babel/preset-typescript"]
}

.tsconfig

{
    "compilerOptions": 
    {
        "target": "es5",
        "sourceMap": true,
        "module": "esnext",
        "jsx": "react",
        "lib": [ "es2015", "es2016", "es2017", "esnext", "dom" ],
        "outDir": "./dist/",
        "noImplicitAny": true,
        "allowJs": true,
        "pretty": false,
        "skipLibCheck": true,
        "esModuleInterop": true,
        "allowSyntheticDefaultImports": true,
        "moduleResolution": "node"
    },
    "include": [
        "./src/**/*",
        "./typings/**/*"
    ]
}

🤔 Expected Behavior

A parcel build, being served statically with the React app loading properly.

😯 Current Behavior

--------------- Upon building and serving the build with minification ---------------

parcel build ./src/client/index.html --no-cache --out-dir ./dist/client  --experimental-scope-hoisting

Loading the app the following error is thrown, caused by lodash requiring the 'buffer' module.

Uncaught Error: Cannot find module 'buffer'
    at u (main.2d3e7cc1.js:1)
    at Object.<anonymous> (main.2d3e7cc1.js:1)
    at main.2d3e7cc1.js:1
    at main.2d3e7cc1.js:1
    at main.2d3e7cc1.js:1

--------------- Building the app without minification. ---------------

parcel build ./src/client/index.html --no-minify --no-cache --out-dir ./dist/client  --experimental-scope-hoisting

Loading the app the following error is thrown.

main.c98ce985.js:40091 Uncaught ReferenceError: $Rd93$var$warning is not defined
    at validateTypeDef (main.c98ce985.js:40091)
    at Object.propTypes (main.c98ce985.js:40074)
    at mixSpecIntoComponent (main.c98ce985.js:40155)
    at createClass (main.c98ce985.js:40447)
    at $I4qe$init (main.c98ce985.js:40657)
    at $YBnz$init (main.c98ce985.js:40971)
    at $oN7R$init (main.c98ce985.js:41276)
    at $jUtL$init (main.c98ce985.js:41638)
    at main.c98ce985.js:41741
    at main.c98ce985.js:42

Also causes another library (redux) to throw

You are currently using minified code outside of NODE_ENV === 'production'.

💁 Possible Solution

🔦 Context

Would like to achieve the same bundle sizes as we did with a webpack build through the use of tree shaking, but it causes the app to not load.

We run a React app, which uses a mix of JS and Typescript.

💻 Code Sample

Kind of hard to provide a sample, it happens in a production running application. Can attempt to create a replication repo.

🌍 Your Environment

| Software | Version(s) |
| ---------------- | ---------- |
| Parcel | 1.12.3
| Node | 8.11.4
| npm/Yarn | 1.9.4
| Operating System | macOS Catalina

[mischnic]

Please remove these settings from your tsconfig: target, sourceMap, module and try again

[1saf]

Upon removal of those settings, I am getting the same results.

[mischnic]

Mh, please create a reproduction repo.

[1saf]

Attempting to re-create a reproduction repo to the best of my ability. The package.json reflects all the dependencies we are using.

https://github.com/1saf/parcel-tree-shaking-bug/tree/master/reproduction-repo

I try to run the build with tree shaking but it fails to build and for the life of me I cannot figure out why. I see that this issue has occurred before with parcel, but was supposedly fixed with 1.10.

../../node_modules/lodash-es/isPlainObject.js does not export 'default'

[mischnic]

Without scope hoisting, I just get a blank screen parcel src/client/index.html.

[1saf]

So, got around that problem with scope hoisting and pushed too. And it works fine with an absolute minimum. Im going to go into our production app and attempt to isolate code in order to get to a point of re-creation. :(

Very sorry.

[1saf]

I have isolated the problem to a library, with all this code commented out in our actual app, the error

main.c98ce985.js:40091 Uncaught ReferenceError: $Rd93$var$warning is not defined

still occurs but I have narrowed it down considerably.

The above error does not appear in a non-minified build with a library 'react-notification-system-redux' being used. But when ran in a non minified build, the error does appear. Very odd. It might be due to conflicting versions of React within the application.

I will report back here once I figure out more, and if so, create a replicable repo.

Thanks for being patient.

[1saf]

Back with an update on this problem.
If you pull the last revision of the repository (please mind that I've cleaned up the structure so you might need to do a clean clone) and setup the environment with the following:
Parcel Build Command:

NODE_ENV=production CLIENT_APPLICATION=client rm -rf ./dist &&  parcel build ./src/client/index.html --no-cache --out-dir ./dist/client --no-minify --no-source-maps --public-url http://localhost:8080/

Dist folder served using superstatic command (while in the dist folder):

superstatic --port 8080 --config ~/superstatic.json

with the following superstatic.json config

{
    "rewrites": [
      {"source":"/**","destination":"/index.html"}
    ]
  }

You'll find that visiting localhost:8080/dashboard will give you the expected output, a bunch of console logs and the yoyoyo being rendered.

With the parcel build with hoisting however,

NODE_ENV=production CLIENT_APPLICATION=client rm -rf ./dist &&  parcel build ./src/client/index.html --no-cache --out-dir ./dist/client --no-minify --no-source-maps --public-url http://localhost:8080/ --experimental-scope-hoisting

You'll find that visiting the same route does not render or log anything. This actually sort of ties back as close as I can connect it to our production app, let me explain. On a scope hoisted build parcel, as youve told me generates its own unique identifiers in the compiled code. For the above repository youll find on a scope hoisted build the following snipper at the end of the generated code for Dashboard.js

({}).__esModule = true;
return {
  "Bt3S": {}
};
});

This unique export identifier is not being referenced into the generated main.js on a scope hoisted build.

NOW, with our production app, on the same scenario without the scope hoisted build, works fine. But with it hoisted, the entry component like above DOES export the following at the bottom, within the generated code:

if (typeof exports === "object" && typeof module !== "undefined") {
  // CommonJS
  module.exports = $wJJ$exports;
} else if (typeof define === "function" && define.amd) {
  // RequireJS
  define(function () {
    return $wJJ$exports;
  });
}

$wJJ$exports.__esModule = true;
return {
  "5wJJ": $wJJ$exports
};
});

And within main.js it is referenced appropriately:

var $NDMP$export$default = function (store) {
  return {
    path: 'signup',
    onEnter: $N1LQ$export$ensureNotLoggedIn(store),
    getComponent: function getComponent(nextState, cb) {
      var Signup = $ccIB$$interop$default.d.lazy(function () {
        return ($DEAj$init(), $DEAj$exports)([["SignupContainer.a44a8c51.js", "5wJJ"], "5wJJ"]);
      });
      cb(null, Signup);
    }
  };
};

But when I serve the app i am met with

main.a5a6646c.js:4358 Error: Cannot find module '5wJJ'
    at localRequire (main.a5a6646c.js:33)
    at localRequire (main.a5a6646c.js:17)
    at main.a5a6646c.js:25261

Any ideas? I am doing something blatantly wrong? (i get that feeling)
At this point I am trying my hardest to get a reproduction repo for you guys but I thought I'd provide an update which could shed some light for both of us.

[1saf]

Another update from me, for those who are wondering what solved the issue for me above. It was due to a css file which defined a font face which was loading eot's and ttf's as fallbacks.
After making sure it only used .woff the issues above disappeared!

The issue now however, is that the prop-checking of react is messing with the build. Pulling the same recreation repository above, and serving with superstatic as in the above comment the following error is thrown:

main.95c56c48.js:186 Uncaught ReferenceError: $ns$var$has is not defined
    at $ns$var$checkPropTypes (main.95c56c48.js:186)
    at validatePropTypes (main.95c56c48.js:2115)
    at Object.createElementWithValidation [as createElement] (main.95c56c48.js:2302)
    at Root.get (main.95c56c48.js:39768)
    at Root.render (main.95c56c48.js:39755)
    at finishClassComponent (main.95c56c48.js:22453)
    at updateClassComponent (main.95c56c48.js:22408)
    at beginWork$1 (main.95c56c48.js:24128)
    at HTMLUnknownElement.callCallback (main.95c56c48.js:4408)
    at Object.invokeGuardedCallbackDev (main.95c56c48.js:4457)

If I step into the compiled main.js finding the reference $ns$var$has which is just a reference to hasOwnProperty is being called before it is defined. So it is not being hoisted appropriately?
@mischnic

[1saf]

I have managed to re-create the same above issue with a react parcel starter library with the latest parcel and react versions as well.

https://github.com/1saf/react-parcel-starter

built with

rm -rf ./dist && NODE_ENV=staging CLIENT_APPLICATION=client  parcel build ./src/index.html --no-cache --out-dir ./dist/ --no-minify --public-url http://localhost:2000/ --experimental-scope-hoisting

and served with

superstatic --port 2000 --config ~/superstatic.json

[mischnic]

I hope this would never occur in the "wild" (when I first discovered this a few months ago):

If you set NODE_ENV=staging, the development version of React is built together with prop-types:

// https://github.com/facebook/prop-types/blob/v15.7.2/checkPropTypes.js

if (process.env.NODE_ENV !== 'production') { // is in init function
  var has = Function.call.bind(Object.prototype.hasOwnProperty);
  // ...
}

function checkPropTypes(typeSpecs, values, location, componentName, getStack) { // is hoisted to toplevel
  if (process.env.NODE_ENV !== 'production') {
    ...
      if (has(typeSpecs, typeSpecName)) { // has will be an undefined variable
        ...

Scope hoisting cannot statically analyze the if statement at the top and creates an init function that is called the first time the file is imported:

function $Qo3t$var$checkPropTypes(typeSpecs, values, location, componentName, getStack) {
    // ...
    if ($Qo3t$var$has(typeSpecs, typeSpecName)) {
        // ...
    }
}


function $Qo3t$init() {
    // ...
    var $Qo3t$var$has = Function.call.bind(Object.prototype.hasOwnProperty);
}

Because of the very disadvantageous rules regarding the scope of var, var has at the top is also accessible outside of the if statement. We'd really need to "pull out" these var declarations from the statement to the toplevel scope as well.

(I don't know if you're aware of this, but NODE_ENV=staging essentially does a React dev build because they check if(process.env.NODE_NV !== "production), this is very different from NODE_ENV=production)

[1saf]

Yeah I figured this would be the problem after going through the React code as well, especially when I saw the production if statement.

What I'm wondering now if this a priority issue (or if this is an issue at all), that I should wait before finalising a migration to Parcel. The issue mainly being able to test a scope hoisted build in our staging environment which becomes difficult since the node environment influences the code.

A hack could possibly be, to make the code reference a different env var, which I guess could work, allowing Parcel to build a production hoisted application and also preventing the React error from being thrown.

EDIT:
I've also encountered some other issues with tree-shaking, should I create new issues for those seperately - or should I include them in the thread here?

[mischnic]

This is fixed in Parcel 2 by https://github.com/parcel-bundler/parcel/pull/4282