Rhino

Rhino

34M Downloads

``var`` scoping is completely wrong

Fuyukai opened this issue ยท 4 comments

commented

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.

commented

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.

commented

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 ^^;

commented

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.

commented

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.