Scope hoisted ES6 classes fail if minified

[postspectacular]

🐛 bug report

I think this is related to #2041, but showing the same symptom in a slightly different context. I.e. It seems that when using scope hoisting and minification, any class method cannot refer to its own class, i.e. to create a copy of itself:

export class Foo {
    constructor(x) {
        this.x = x;
    }
    copy() {
        // this call fails currently...
        return new Foo(this.x);
    }
}

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

// in package.json
"browserslist": ["since 2018"]

🤔 Expected Behavior

The above code should execute as expected (and DOES work if minification is disabled), but throws a runtime error due to wrong naming resolution in the minified version:

😯 Current Behavior

The relevant parts of the built, non-minified version are:

var $wwEC$exports = {};

class $wwEC$export$Foo {
    constructor(x) {
        this.x = x;
    }

    copy() {
        return new $wwEC$export$Foo(this.x);
    }

}

$wwEC$exports.Foo = $wwEC$export$Foo;
const $wwEC$var$a = new $wwEC$export$Foo(42).copy();

However, once minified, this turns into and the generated $wwEC$export$Foo does NOT get properly replaced:

var a = {};
class b {
    constructor(o) {
        this.x = o
    }
    copy() {
        return new $wwEC$export$Foo(this.x)
    }
}
a.Foo = b;
const c = new b(42).copy();

💁 Possible Solution

Not sure if this is an issue w/ Terser or Parcel... Right now the only workaround for some cases is to delegate to separate functions, e.g. to implement the above .copy() operation:

export class Foo {
    constructor(x) {
        this.x = x;
    }
    copy() {
        // this works!
        return foo(this.x);
    }
}

export const foo = (x) => new Foo(x);

🔦 Context

It's a pretty common scenario for class methods to refer to its own class by name and right now this issue prevents any package containing such cases from using scope hoisting, which is a real shame!

💻 Code Sample

See above.

🌍 Your Environment

Software	Version(s)
Parcel	1.11.0
Node	11.5.0
npm/Yarn	1.12.3
Operating System	OSX

[mischnic]

It works without "browserslist": ["since 2018"], so Parcel doesn't handle the ES6 classes correctly (which would otherwise be transpiled by Babel)

[postspectacular]

@mischnic That's interesting, though I really don't want Babel to transpile down to ES5. So it'd be great to have this working w/ ES6 outputs too... Happy to take a look myself, just not quite sure where to start. Any pointers?

[mischnic]

Happy to take a look myself, just not quite sure where to start. Any pointers?

This call here

parcel/packages/core/parcel-bundler/src/scope-hoisting/hoist.js

Line 146 in 08398bd

rename(scope, name, newName);

renames the variables. It seems to miss some occurences.
Minimal example:

class Foo {
    copy() {
        return new Foo();
    }
}

new Foo();

[mischnic]

For some reason, the reference to the class inside of copy isn't listed in referencePaths

parcel/packages/core/parcel-bundler/src/scope-hoisting/renamer.js

Line 21 in 08398bd

for (let path of binding.referencePaths) {

Replacing

parcel/packages/core/parcel-bundler/src/scope-hoisting/hoist.js

Line 146 in 08398bd

rename(scope, name, newName);

with scope.rename(name, newName) fixes the issue. (There was a similar issue before)

Babel works correctly:

const core = require('@babel/core');
const traverse = require('@babel/traverse').default;

const code = `
class Foo {
    copy() {
        return new Foo();
    }
}

new Foo();
`;

const { ast } = core.transformSync(code, {ast: true});

traverse(ast, {
  Program(path) {
    console.log(path.scope.getBinding('Foo').referencePaths.length);
  }
});

[mischnic]

@devongovett The babel bugs become more even more obscure:
The babel code below outputs (the inner Foo isn't renamed because it's missing in referencePaths):

class abc {
  copy() {
    return new Foo();
  }

}

new abc();

Remove scope.crawl() and it works just fine.
babel's scope.rename() also works properly in both cases.

const core = require("@babel/core");
const traverse = require("@babel/traverse").default;
const generator = require("@babel/generator").default;

const code = `
class Foo {
    copy() {
        return new Foo();
    }
}
new Foo();
`;

const { ast } = core.transformSync(code, { ast: true });

traverse(ast, {
	Program(path) {
		const { scope } = path;

		scope.crawl();

		// parcel's rename algorithm:
		// ------------------------------
		let binding = scope.getBinding("Foo");

		// Rename all references
		for (let path of binding.referencePaths) {
			if (path.node.name === "Foo") {
				path.node.name = "abc";
			}
		}

		// Rename binding identifier, and update scope.
		scope.removeOwnBinding("Foo");
		scope.bindings["abc"] = binding;
		binding.identifier.name = "abc";
	}
});

console.log(generator(ast).code);

[devongovett]

This is possibly fixed by #2986.

[mischnic]

This is possibly fixed by #2986.

Yes. The other issue #2981 is actually a duplicate of this one.

[postspectacular]

Hi @mischnic @devongovett - thanks for this!!! Is that fix going to be released anytime soon or will be part of v2?