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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class renaming breaks const usage #3222

Closed
AshleySchaeffer opened this issue Jul 12, 2023 · 4 comments
Closed

Class renaming breaks const usage #3222

AshleySchaeffer opened this issue Jul 12, 2023 · 4 comments

Comments

@AshleySchaeffer
Copy link

I'm trying to bundle some JS produced by a third-party and the authors use of a const seems to be incompatible with esbuild.

Having gone through #510 I can't seem to find a way of getting a pattern like the following to work:

const Foo = {
    components: {}

    init: function () {
         Console.log("Foo.init")
    }
}

Foo.init()

Foo.components.Bar =  {
    someval: "data"

    init(): function () {
         Console.log("Foo.components.Bar.init")
    }
}

The outputted JS is:

var Foo2 = {
    components: {}

    init: function () {
         Console.log("Foo.init")
    }
}

Foo2.init()

Foo.components.Bar =  {
    someval: "data"

    init() {
         Console.log("Foo.components.Bar.init")
    }
}

This results in the cast to Foo.components.Bar to fail as Foo hasn't been declared as it's been renamed to Foo2. I've tried keep-names but from my limited understanding of what that flag does, it's not going to help me here. I can't find any other solutions from docs.

Apologies if I'm missed something obvious. Annoyingly this use of a const occurs throughout a larger JS code base so refactoring would take considerable amounts of work. Any help would be greatly appreciated.

@evanw
Copy link
Owner

evanw commented Jul 12, 2023

I cannot reproduce your issue because the code you have provided is not valid JavaScript (there's a missing comma among other problems):

const Foo = {
    components: {}

    init: function () {
         Console.log("Foo.init")
    }
}

Foo.init()

Foo.components.Bar =  {
    someval: "data"

    init(): function () {
         Console.log("Foo.components.Bar.init")
    }
}

Please demonstrate your issue by getting it to happen on https://esbuild.github.io/try/ and then pasting the link here. I'm marking this issue as unactionable until a way to reproduce this issue is provided.

@AshleySchaeffer
Copy link
Author

AshleySchaeffer commented Jul 12, 2023

Thanks for the link and apologies for the broken example, I was working pretty late.

After trying to reproduce the issue at the link you provided, the FooCore var declaration (I've renamed some variables from the broken example I provided above) is not renamed to FooCore2 which is is correct. However, when using the setup described below it does get renamed to FooCore2 causing an error when FooCore.components.Bar is declared.

In foo.core.js:

const FooCore = {
	components: {},

	init: function () {
		const that = this;
                Console.log("Foo.init");

	}
}

FooCore.init()

In foo.bar.js:

FooCore.components.Bar =  {
    someval: {},

    init: function () {
         Console.log("Foo.components.Bar.init")
    }
}

In app.js:

import "./foo.core.js"
import "./foo.bar.js"

The bundled output produced looks like this:

(() => {
  // js/foo.core.js
  // ### This should be FooCore ###
  var FooCore2 = {
    components: {},
    init: function() {
      const that = this;
      Console.log("Foo.init");
    }
  };
  FooCore2.init();

  // js/foo.bar.js
  FooCore.components.Bar = {
    someval: {},
    init: function() {
      Console.log("Foo.components.Bar.init");
    }
  };
})();

If I put the contents of foo.core.js and foo.bar.js in app.js and remove the need to import, the build output is not broken as shown in the at the link you provided:

https://esbuild.github.io/try/#YgAwLjE4LjExAC0tYnVuZGxlIC0tdGFyZ2V0PWVzMjAxNwBlAGVudHJ5LmpzAGNvbnN0IEZvb0NvcmUgPSB7Cgljb21wb25lbnRzOiB7fSwKCglpbml0OiBmdW5jdGlvbiAoKSB7CgkJY29uc3QgdGhhdCA9IHRoaXM7CiAgICAgICAgICAgICAgICBDb25zb2xlLmxvZygiRm9vLmluaXQiKTsKCgl9Cn0KCkZvb0NvcmUuaW5pdCgpCgpGb29Db3JlLmNvbXBvbmVudHMuQmFyID0gIHsKICAgIHNvbWV2YWw6IHt9LAoKICAgIGluaXQ6IGZ1bmN0aW9uICgpIHsKICAgICAgICAgQ29uc29sZS5sb2coIkZvby5jb21wb25lbnRzLkJhci5pbml0IikKICAgIH0KfQ

@evanw
Copy link
Owner

evanw commented Jul 12, 2023

I see. I believe you may be confused about how modules work in JavaScript. JavaScript does not allow you to reference a local variable from another module directly by name. Instead you need to export that variable from one file and import it into another. So instead of doing this you should do this.

@AshleySchaeffer
Copy link
Author

Ahhhh. Thank you. It's all just clicked. The code base I'm using was originally designed to be used with gulp. Where it takes all the JS and bundles it into one big file, crudely.

Thank you so much for your time on this. I really appreciate it considering it's not an esbuild issue.

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

No branches or pull requests

2 participants