-
Notifications
You must be signed in to change notification settings - Fork 850
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
Update Shell to use Context.VERSION_ES6 by default #1594
Update Shell to use Context.VERSION_ES6 by default #1594
Conversation
Of course it couldn't have been that easy. |
Sorry, I didn't think it would be!
…On Thu, Aug 29, 2024 at 4:19 PM Tony Germano ***@***.***> wrote:
Of course it couldn't have been that easy.
—
Reply to this email directly, view it on GitHub
<#1594 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD7I2YM7DMUQI4PHUM3Q6DZT6T63AVCNFSM6AAAAABNLHVHB6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJZGM4DINZYGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Several tests failed after changing the ShellContextFactory default language version to ES6.
MozillaSuiteTest uses ShellContextFactory (as do some other tests,) but did not explicitly set the version to 1.8, so I'm pushing a change to do that now. Of the handful of failing tests that I looked at, they all used legacy features that are not part of the current javacsript spec (javascript 1.7 generators and let expressions.) I just did a search, and there are quite a few other tests that reference Context.VERSION_1_8. At some point should we try to update what we can to run with VERSION_ES6 or just leave the old tests at the old language versions and expect newer tests and test262 to cover everything in ES6+? |
I think this one is ready now after my last commit if someone wants to review. |
Doing a sanity test myself, but I think that this looks good. @p-bakker do you think that we can have some way to flag PRs / commits that will need extra attention in the release notes? And is this a sufficient reason why the next release should be called "1.8.0"? And should we explore making "VERSION_ES6" the default version for everything? |
I think that this is good. I'm going to leave this open for a few days in case any active users want to try it out themselves. |
Tagged the PR with the 'Potential breaking change' label, so we can look for those when making the release notes and include a warning iMHO this change doesn't warrant a minor version update, as I doubt there's much usage of the shell these days (based on Google search results the shell used to be used to run all kinds of JS tooling that have long since moved to NodeJS or other runtimes). And if there's usage left, I'd think they either aren't likely to upgrade to a newer Rhino version or will use ES6 As for making ES6 the default: personally I rather make a new language version called EcmaScript, which by default doesn't include all non-standard stuff and make that the default, see #1055 (and related #1056, #1057 & #1060) |
@p-bakker are you talking about making the new language version the default elsewhere in the code? The shell has always included the kitchen sink, and I use it quite a bit as a repl for testing both java interop and e4x behavior. Plain javascript in the rhino shell is not very useful, because it doesn't have any of the APIs that allow it to interact with the outside world without java. Rhino has always been presented as a way to script java using javascript. |
My thought was to create an EcmaScript language version that just had plain EcmaScript, nothing else and then there would be flags to opt-in on additional things, like Java Interop, E4X, Continuations etc. Whether or not the Shell would have all those flags turned on our of (by default) would be a decision to be made, but I can see it being useful to have them on by default. But, this is all just my thoughts, as a group we'd have to come to a consensus how to move forward on things like this 🙂 |
I looked at #1055 before I made my last comment, and I like your idea as it applies to embedding, but I do think the shell should at least have java interop turned on by default. Personally, I'd like e4x, too, but if I have to enable that with a command-line option I could live with it (though I also don't see the harm in having it enabled by default in the shell, as it has always been.) And of course there are the shell-only objects that currently get added which would not be part of plain ecmascript. But until we have all of that implemented, my hope would be that we could move forward with this simple PR with the understanding that it will likely get changed again down the road. |
Oh, I definitely agree: update to use ES6 now and if we'll get to the other cases at some point, week see them how to proceed with the language version/setup in the shell as well |
Yes, this makes sense to me now - thanks! |
resolves #1279