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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

--experimental-scope-hoisting not working with rollup build #2981

Closed
katzIdo opened this issue May 6, 2019 · 2 comments 路 Fixed by #2986
Closed

--experimental-scope-hoisting not working with rollup build #2981

katzIdo opened this issue May 6, 2019 · 2 comments 路 Fixed by #2986

Comments

@katzIdo
Copy link

katzIdo commented May 6, 2019

馃悰 bug report

in the following example you can see a problem with --experimental-scope-hoisting when using rollup build cjs

馃 Expected Behavior

Given a project with lerna and multiple packages
package build script
hoistexample-rollup rollup rollup src/index.js --file lib/index.js --format cjs
hoistexample-ts ts tsc --module commonjs --target es5 --outDir lib
When a main package imports the packages 
And bundle it all with 
 # parcel build -o build index.js  --experimental-scope-hoisting --no-minify 
Then  parcel change the variables name of the rollup build correctly as it does for ts build

馃槸 Current Behavior

Given a project with lerna and multiple packages
package build script
hoistexample-rollup rollup rollup src/index.js --file lib/index.js --format cjs
hoistexample-ts ts tsc --module commonjs --target es5 --outDir lib
When a main package imports the packages 
And bundle it all with 
 # parcel build -o build index.js  --experimental-scope-hoisting --no-minify 
Then parcel change the variables name of the rollup build incorrectly 
And runtime throw exception of ReferenceError

馃拋 Possible Solution

you can see from the example
that the problem occurs when doing the export in one line.

export class failurtestHositingRollup2 {
 static create() {
   return new failurtestHositingRollup2();
 }
};

export default class failurtestHositingRollup {
 static create() {
   return new failurtestHositingRollup();
 }
}

馃捇 Code Sample

parcel bundle with ts import build:

var $sbuw$var$testHositing =
/** @class */
function () {
  function testHositing() {}

  testHositing.create = function () {
    return new testHositing();
  };

  return testHositing;
}();

parcel bundle with rollup import build:

class $gcK$var$failurtestHositingRollup {
  static create() {
    return new failurtestHositingRollup();
  }

}

馃實 Your Environment

Software Version(s)
parcel-bundler 1.6.1
Node v10.15.0
Yarn 1.12.3
Operating System MAC
@devongovett
Copy link
Member

I believe this is a bug in Babel's scope tracking. See here for a reduced test case: https://astexplorer.net/#/gist/f1cb0faddd7b06a58e34f59051c78c39/ebab99e1bb5f0be982245db354e9b4abd48c9741.

The console.log in that example outputs 1 reference to Test (the export declaration) when it should output 2 (including the reference in create). If you comment out the call to path.scope.crawl() however, it does output 2 correctly, so something about calling crawl again is messing it up. I tried removing our call to crawl in the scope hoisting code, but that broke other tests. Not sure what's going on there.

@devongovett
Copy link
Member

Managed to work around the issue by clearing babel's scope cache before we re-crawl. #2986

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants