parcel-bundler / parcel

The zero configuration build tool for the web. 📦🚀
https://parceljs.org
MIT License
43.29k stars 2.26k forks source link

Scope hoisting sometimes causes runtime error "$exports is not defined" #6071

Open astegmaier opened 3 years ago

astegmaier commented 3 years ago

🐛 bug report

I went to test out the latest version of parcel in a real-life app (congrats on hitting beta 2!) and I'm hitting an issue with scope-hoisting. Importing anything from the @fluentui/react project (with scope-hoisting on) creates this error at runtime:

Uncaught ReferenceError: $7246c9565e62267fb2e857060ee548f8$exports is not defined

I've perused a few recent issues that reported a similar runtime error (e.g. #5892 and #6008), and although the symptom is the same, the minimal repro seems quite different.

🎛 Real-life repro

You can try it yourself in the parcel-repro-real package of this repo

index.js

import { Overlay } from "@fluentui/react"; // Or any other component from the library
console.log("here is Overlay: ", Overlay);

console command

parcel build index.js --no-optimize

🎛 Simplified repro

This was my best attempt at getting the simplest possible simulation of how @fluentui/react is structured in the way that causes the issue. It's a bit of a "perfect storm" of factors that interact with each other to cause the error - I tried to annotate all the places where if things had been slightly different, there would be no issue. Even though it's a very specific set of causes, I can't find anything "wrong" with it at a conceptual level - it seems like it should work (and it does in webpack). You can try it yourself in the parcel-repro-simple package of this repo

Parcel App

index.js

import { aStringExport } from "simple-dependency";
console.log(aStringExport);

console command

parcel build index.js --no-optimize

simple-dependency

package.json

{
  "name": "simple-dependency",
  "main": "index.js",
  "sideEffects": false // changing to "true" fixes the runtime error for parcel (although it would be functionally incorrect).
}

index.js

// changing the style here to "export { thing } from './file'" fixes the runtime error for parcel (webpack works with either style).
export * from "./sum";
export * from "./product"; // removing this export fixes the runtime error for parcel

sum.js

import { add } from "simple-dependency2";
export const sum = add(3, 3); // unused in parcel project
export const aStringExport = "here is a string exported from simple-dependency"; // the only thing imported by parcel

product.js

import { multiply } from "simple-dependency2";
export const product = multiply(3, 3); // unused in parcel project

simple-dependency2

package.json

{
  "name": "simple-dependency2",
  "main": "index.js",
  "sideEffects": [
    "index.js" // changing to "false" or "true" fixes the runtime error (although those would be functionally incorrect)
  ]
}

index.js

export * from "./math"; // replacing this with "export { add } from 'math/add'", etc. (thus bypassing math/index.js) fixes the runtime error for parcel
console.log("here is a side effect from simple-dependency2!");

math/index.js

export { add } from "./add";
export { multiply } from "./multiply";

math/add.js

// only used by simple-dependency, not by parcel app:
export function add(num1, num2) {
  return num1 + num2;
}

math/multiply.js

// only used by simple-dependency, not by parcel app:
export function multiply(num1, num2) {
  return num1 * num2;
}

🤔 Expected Behavior

Both scenarios compile and run without blowing up. I tested the latest version of webpack, and it is able to tree-shake these projects without error.

😯 Current Behavior

Both scenarios build without errors, but blow up at runtime with a console error similar to:

Uncaught ReferenceError: $7246c9565e62267fb2e857060ee548f8$exports is not defined

The interesting lines (unminified) build output for the simple case looks like this:

  //...
  // ASSET: simple-dependency2/index.js
  var $afa56de6632f40a42afd696abe8b6333$exports = {};
  function $8151e1fe52185a57218348701c630f90$export$add(num1, num2) {
    return num1 + num2;
  }
  // This line blows up, because the second parameter ($7245...) is not defined
  $parcel$exportWildcard($afa56de6632f40a42afd696abe8b6333$exports, $7246c9565e62267fb2e857060ee548f8$exports);
  console.log("here is a side effect from simple-dependency2!");
  //...

💁 Possible Solution

I'm unfamiliar with how scope-hoisting is currently implemented, but I'd love to dive in and help make the fix if I could get a few pointers/ideas of how to start grokking it.

🔦 Context

There are a bunch of teams at MSFT that use the @fluentui/react library - I'd love to share all the goodness of parcel2 with them, so fixing this issue would be great!

💻 Code Sample

Repo to demonstrate the "simple", "real" and webpack scenarios discussed above: https://github.com/astegmaier/parcel2-scope-hoisting-bug

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.632
Node 14.15.4
npm/Yarn 1.22.10
Operating System OS X 10.15.7
astegmaier commented 3 years ago

I did some debugging, and I wanted to share what I found so far in case it's helpful.

The final reason why this is blowing up at runtime is because of the presence of this statement in the bundle, where the second parameter ($724...) is never defined:

$parcel$exportWildcard($afa56de6632f40a42afd696abe8b6333$exports, $7246c9565e62267fb2e857060ee548f8$exports);

There would be two ways of preventing this runtime error:

  1. Preventing the $parcel$exportWildcard... statement from showing up in the bundle in the first place
  2. Allowing it to show up, but making sure that the $724... variable is defined.

It seems like, at least in this case, number 1 would be preferrable (since it is unused), so I set about seeing what code was controlling this.

Within @parcel/scope-hoisting, there is a function called needsExportsIdentifierForAsset that seems to be in charge of this decision. When it is called with the simple-dependency2/index.js asset, it currently decides that the exports identifier does need to be included in the bundle because of this conditional check, which was added in PR #4815:

https://github.com/parcel-bundler/parcel/blob/f8981bfb142631f286c98fe4aaa0adfe674ce057/packages/shared/scope-hoisting/src/link.js#L384-L388

This condition runs for each "incoming dependency" of the asset (in this case, simple-dependency/sum.js and simple-dependency/product.js), and it returns true for simple-dependency/product.js.

This is caused by the fact that resolveSymbol returns { identifier: false ... } when it is called with asset: simple-dependency/product.js and symbol: multiply.

This, in turn, is caused by the isDependencySkipped check within resolveSymbol, which returns true when it is called for simple-dependency2/math/multiply.js:

https://github.com/parcel-bundler/parcel/blob/f8981bfb142631f286c98fe4aaa0adfe674ce057/packages/core/core/src/BundleGraph.js#L1169-L1173

Which (finally!) is caused by the fact that the node for simple-dependency2/math/multiply.js is has the property excluded: true.

I'm still not sure the right fix here (scope hoisting is complicated 🤯), but hopefully that sheds some light on what is going on. @devongovett - the conditional check in needsExportsIdentifierForAsset was part of your PR #4815 - do you have any thoughts about the way it should be working in light of this issue?

mischnic commented 3 years ago

I think the problem lies here:

// If one of the symbols imported by the dependency doesn't resolve, then we need the // exports identifier to fall back to.

"doesn't resolve" would correspond to null and undefined, but if it's false, we don#t need/want that fallacb

https://github.com/parcel-bundler/parcel/blob/f8981bfb142631f286c98fe4aaa0adfe674ce057/packages/core/types/index.js#L1049-L1051

So resolveSymbol(...).identifier == null would make more sense to me instead of !...(but I haven't tested this).

Having said that, this already works correctly in a WIP branch that restructured scope hoisting quite a lot.

ericclemmons commented 2 years ago

Thanks for everyone sharing on this thread. I just updated to parcel@2.0.1 and get a similar error within my yarn workspaces:

Screen Shot 2021-11-23 at 7 36 26 PM

The offending line comes from @aws-amplify/ui-react/dist/index.js:

var $fWTuU$awsamplifyui = require("@aws-amplify/ui");
var $fWTuU$react = require("react");
var $fWTuU$radixuireactid = require("@radix-ui/react-id");
var $fWTuU$xstatereact = require("@xstate/react");
var $fWTuU$reactgeneratecontext = require("react-generate-context");
var $fWTuU$classnames = require("classnames");
var $fWTuU$autoprefixer = require("autoprefixer");
var $fWTuU$postcssjs = require("postcss-js");
var $fWTuU$lodashdebounce = require("lodash/debounce");
var $fWTuU$radixuireactaccordion = require("@radix-ui/react-accordion");
var $fWTuU$radixuireactdropdownmenu = require("@radix-ui/react-dropdown-menu");
var $fWTuU$radixuireactslider = require("@radix-ui/react-slider");
var $fWTuU$radixuireacttabs = require("@radix-ui/react-tabs");
var $fWTuU$awsamplify = require("aws-amplify");
var $fWTuU$qrcode = require("qrcode");

function $parcel$export(e, n, v, s) {
  Object.defineProperty(e, n, {get: v, set: s, enumerable: true, configurable: true});
}
function $parcel$exportWildcard(dest, source) {
  Object.keys(source).forEach(function(key) {
    if (key === 'default' || key === '__esModule' || dest.hasOwnProperty(key)) {
      return;
    }

    Object.defineProperty(dest, key, {
      enumerable: true,
      get: function get() {
        return source[key];
      }
    });
  });

  return dest;
}
function $parcel$interopDefault(a) {
  return a && a.__esModule ? a.default : a;
}

$parcel$export(module.exports, "components", () => $f0cdf2d9e4d80ac9$exports);
$parcel$export(module.exports, "primitives", () => $f0cdf2d9e4d80ac9$exports);
$parcel$export(module.exports, "defaultTheme", () => $e29a64e57a51ca74$re_export$defaultTheme);
$parcel$export(module.exports, "createTheme", () => $e29a64e57a51ca74$re_export$createTheme);

The problem is, the @aws-amplify/ui package uses a different hash:

var $542f5d59f79a26f8$exports = {};

$parcel$export($542f5d59f79a26f8$exports, "defaultTheme", () => $542f5d59f79a26f8$export$164de7ab8df77ef0);
$parcel$export($542f5d59f79a26f8$exports, "createTheme", () => $54a00421d9011a65$export$25d302a5b900a763);
mischnic commented 2 years ago

@ericclemmons Can you provide a reproduction (maybe in a new issue)? In a basic react app, these $parcel$export also look different for me, what amplify package versions are you using?

I'm also curious how you are using Parcel with React Native 😄

ericclemmons commented 2 years ago

@mischnic I've uploaded my branch to reproduction steps to https://github.com/aws-amplify/amplify-ui/pull/871.

I suspect the hashes are different because of symlinks due to yarn workspaces. So these aren't live packages (yet). I can open an issue, but I suspect that parcel may not be the best option for creating a library bundle since .native.js files need to exist for the Metro bundler to prefer over Node/Browser.

FlorianKoerner commented 2 years ago

Some here. Simplified example from the repo https://github.com/dicebear/dicebear/tree/5.0:

Input:

import * as adventurer from '@dicebear/adventurer';

export {
  adventurer,
};

Output:

var $iqXQb$dicebearadventurer = require("@dicebear/adventurer");

function $parcel$export(e, n, v, s) {
  Object.defineProperty(e, n, {get: v, set: s, enumerable: true, configurable: true});
}

$parcel$export(module.exports, "adventurer", function () { return $5089fad42d572689$re_export$adventurer; });
Calcifer1001 commented 1 year ago

Hello. I'm having this same issue. Is there any provisional solution? Tell me if I can help you find one

cokolele commented 1 year ago

Still having this issue. My code:

import {
    select,
    scaleLinear,
    scaleLog,
    extent,
    axisBottom,
    axisLeft,
    line
} from "d3"

const d3 = {
    select,
    scaleLinear,
    scaleLog,
    extent,
    axisBottom,
    axisLeft,
    line
}

interestingly, only some properties reference to "...$exports". Some get translated to functions:

{ extent:$8a929823409c3d58$exports.extent, axisBottom:function(e){return Qn(3,e)} }