website icon indicating copy to clipboard operation
website copied to clipboard

equally named private methods in different files break on merge

Open Cxarli opened this issue 4 years ago • 4 comments

Bug Report

If you have two files, each with a different class, but the classes share a method with the same name, the merged output file breaks because of name overlap.

  • [x] I would like to work on a fix!

Input Code

// A.js
class A {
    constructor() {}
    public() { return this.#private(); }
    #private() { console.log('A private'); }
}

// B.js
class B {
    constructor() {}
    public() { return this.#private(); }
    #private() { console.log('B private'); }
}

// main.js
// Expected: "A private"
// Actual:   "B private"
(new A).public();

Also see this repo

Run Command npx babel A.js B.js main.js -o out.js

Output Code

(slightly trimmed to make it clearer what the issue is, not actual output)

var _private = new WeakSet();
class A {
  constructor() { _private.add(this); }
  public() { return _classPrivateMethodGet(this, _private, _private2).call(this); }
}
var _private2 = function _private2() { console.log('A private'); };

// this ID is reused
var _private = new WeakSet();
class B {
  constructor() { _private.add(this); }
  public() { return _classPrivateMethodGet(this, _private, _private2).call(this); }
}
// this ID is reused
var _private2 = function _private2() { console.log('B private'); };

Expected behavior The second pair should get unique names, so they don't clash, which would make the result "A private"

Babel Configuration

  • Filename: babel.config.js
module.exports = {
    plugins: [
        [require("@babel/plugin-proposal-class-properties")],
        [require("@babel/plugin-proposal-private-methods")],
    ],
};

Environment

  Binaries:
    Node: 14.7.0 - /usr/bin/node
    Yarn: 1.22.4 - /usr/bin/yarn
    npm: 6.14.7 - /usr/bin/npm
  npmPackages:
    @babel/cli: ^7.10.5 => 7.10.5 
    @babel/core: ^7.11.1 => 7.11.1 
    @babel/plugin-proposal-class-properties: ^7.10.4 => 7.10.4 
    @babel/plugin-proposal-private-methods: ^7.10.4 => 7.10.4 

Possible Solution Technically you can first concat the files before putting it through babel with npx babel <(cat A.js B.js main.js) -o out.js which means they are in the same scope and get unique functions and everything works. However, then you lose all source information (file, line) in case of errors.

Cxarli avatar Aug 07 '20 17:08 Cxarli

Hey @Cxarli! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

babel-bot avatar Aug 07 '20 17:08 babel-bot

This is expected. @babel/cli simply concatenate compiled JavaScript files and the semantic may change after codes adding up. In your case, I would suggest a bundler like Rollup of WebPack that can ensure the module-scoped variables does not interfere with other modules.

We should state clearly such limitations on https://babeljs.io/docs/en/babel-cli#compile-directories

JLHwung avatar Aug 10 '20 19:08 JLHwung

Hey @Cxarli! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

babel-bot avatar Aug 10 '20 19:08 babel-bot

I guess it can be expected if you manually write duplicate global variables/functions. However, this issue seems to be about code generated by @babel/plugin-proposal-private-methods. I wasn't sure where to report this issue, but I thought babel/babel was the right repo for that, given that the plugin seems to be implemented there.

I'm not sure what could be a waterproof solution here, but I was thinking maybe the plugin could prepend the class name in front of the WeakSet and the private method converted to global function, so they are named like _A_private and _B_private. This would still clash when you have different files declaring a class with the same name, but that issue can be expected.

Cxarli avatar Aug 17 '20 17:08 Cxarli