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

var scoping is completely wrong #36

Open
Fuyukai opened this issue Apr 5, 2023 · 4 comments
Open

var scoping is completely wrong #36

Fuyukai opened this issue Apr 5, 2023 · 4 comments

Comments

@Fuyukai
Copy link
Contributor

Fuyukai commented Apr 5, 2023

Input code:

if (1) { var thing = "exists!"; }
console.info(thing);  /// or print for Mozilla rhino. not really the point

Node output: exists!
Mozilla Rhino output: exists!
This repo output: dev.latvian.mods.rhino.EcmaError: ReferenceError: "thing" is not defined.

In ECMA var is function scoped. Is this a bad idea? Yes. Should you use var over let/const? Definitely not. Yet, things such as Babel will use var anyway e.g. to polyfill in varargs.

Discovered whilst investigating #35.

@erisdev
Copy link

erisdev commented Apr 9, 2023

Aha, this is biting me in the ass too. Trying to use TypeScript and their helper functions rely on function scoped var as well.

Is this a regression? I don't recall this coming up on previous versions. I was using TypeScript + rollup to bundle scripts on Minecraft 1.16 without issue.

@MaxNeedsSnacks
Copy link
Member

MaxNeedsSnacks commented Apr 9, 2023

It is an "intentional" regression, as in someone (cough @LatvianModder) thought "oh you shouldn't be using var anyways, and this is not supposed to be a faithful brwoser-compatible js engine, it's meant for a block game and it's meant to be usable by noobs", and the way var was done upstream has also been broken-ish in upstream Rhino, so... that's where we've ended up ^^;

@Fuyukai
Copy link
Contributor Author

Fuyukai commented Apr 9, 2023

I think generally enough semi-recentish ES functions are implemented that you don't need to resort to a lot of polyfills or transformations (if you're just writing regular JS rather than TS) so I have mostly worked around this issue (varargs would be nice, though). The only babel plugins I have enabled are classes and numeric separators now.

I did have a look at what might cause this whilst fixing other bugs and I have a good idea of where to look but I'm more focused on using KubeJS right now so I'll likely get back to it in a few weeks time and have a proper attempt then.

@erisdev
Copy link

erisdev commented Apr 10, 2023

It does kind of make life hard for folks who want nice things like TypeScript and modern JS features, because it's kind of all or nothing. Transpiling to ES5 with TS or Babel used to work great, but now KubeJS chokes because of the scoping changes. I don't see where this helps anybody.

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

No branches or pull requests

3 participants