oracle / graaljs

GraalJS – A high-performance, ECMAScript compliant, and embeddable JavaScript runtime for Java
https://www.graalvm.org/javascript/
Universal Permissive License v1.0
1.82k stars 191 forks source link

Q: ANNEXB disable story #847

Open ecki opened 2 months ago

ecki commented 2 months ago

There is a lot of harsh critic of the legacy functions and behavior of AnnexB in AnnexB, so it feels natural to disable it (in standalone script engine). However the extend is not completely clear from the doc, and before I search the usage of the option in the source, is there some mission statement?

I would feel a bit uneasy in removing escape() and unescape(), can we make the disable option more granular and separate the syntax improvements from the replaced built in functions and the less critical ones? js.disable-AnnexB-html or js.enable-AnnexB-escaling or some such?

https://tc39.es/ecma262/#sec-additional-ecmascript-features-for-web-browsers

All of the language features and behaviours specified in this annex have one or more undesirable characteristics and in the absence of legacy usage would be removed from this specification…

These features are not considered part of the core ECMAScript language. Programmers should not use or assume the existence of these features and behaviours when writing new ECMAScript code. ECMAScript implementations are discouraged from implementing these features unless the implementation is part of a web browser

iamstolis commented 2 months ago

is there some mission statement?

AnnexB exists for legacy reasons only i.e. to keep the code (that uses the features already) working.

can we make the disable option more granular

I am sorry, I don't understand what use-case would more granular options for AnnexB solve exactly. If you are not sure that the code that you execute does not depend on AnnexB features then it is better to keep them enabled. Otherwise, it is a good idea to disabled AnnexB features completely, no?

ecki commented 2 months ago

Well, if Graal thinks this is for legacy reasons (and I would fully agree, especially given the actual changes mostly make sense), why not disable it by default like the specification suggests?

I would like to do so, but it might be needed for one feature or the other. So beeing able not to have to use the more harmful parts (or the more obvious parts like the html compatibility) seperately looks like a good alternatively to keep it entirely enabled.

(Having said that it would be good to know what stuff is really turned off, like is it altering the syntax or only removing the String.properties, or…)

iamstolis commented 2 months ago

Well, if Graal thinks this is for legacy reasons ... why not disable it by default like the specification suggests?

For the same reason these features are not disabled in other JavaScript engines/runtimes: compatibility. There is a lot of code that is using some of these features. We do not want to break this code. Moreover, it would be non-trivial for an average user to map the problem caused by a disabled non-annexB feature to AnnexB (option).

Personally, I find AnnexB features more ugly than harmful. JavaScript would be just nicer/cleaner without them.

Having said that it would be good to know what stuff is really turned off

js.annex-b=false option is supposed to disable all the behavior described in AnnexB. Of course, there may be some bugs.

ecki commented 2 months ago

BTW I think the risk is very low, without DOM or Web APIs and only simple modules being supported in the standard JS context I don’t think it would hurt to turn annex-b off by default like the spec wants us to (the only thing which I would worry about is escape/unescape, that’s why I asked)