Javascript es6 migration

[ericvergnaud]

No description provided.

[ericvergnaud]

@carocad
Hi,
as I migrated one of my parser to this version, I noticed a number of remaining prototype inheritance fragments when generating lexer and parser. Also noticed a bug due to an incorrect require. Also got annoying warnings related to cyclical imports. I fixed all that.
I also decided to evolve towards esm semantics (which requires a more recent version of node).
It also requires exposing the runtime as a package, which I achieve using npm commands.
Everything works fine on my mac using node 14.12.
But for whatever reason, the travis build hangs when running the test cmd line.
I'm running out of ideas re why that is...
Would you be able to have a look, could be something obvious that I'm not seeing...

[carocad]

hey @ericvergnaud I will take a look later today. I dont have much experience with es6 imports in node.js since they are so new but I can take a look for sure 😉 .

[ericvergnaud]

hey @ericvergnaud I will take a look later today. I dont have much experience with es6 imports in node.js since they are so new but I can take a look for sure 😉 .

Cool thanks
I see a suspicious thing in the build log:

Setting up nodejs (14.13.0-1nodesource1) ...
v8.9.1

as if installing node 14 had no effect on which node is used...

[carocad]

@ericvergnaud if I recall correctly these files are not needed anymore since we dropped the browser test suite in #2755

[ericvergnaud]

Agreed Envoyé de mon iPhone

…

Le 5 oct. 2020 à 00:24, Camilo Roca ***@***.***> a écrit :  @carocad commented on this pull request. In runtime-testsuite/resources/org/antlr/v4/test/runtime/templates/Chrome.test.stg: > @@ -74,7 +74,7 @@ LANotEquals(i, v) ::= <%this._input.LA(<i>)!=<v>%> TokenStartColumnEquals(i) ::= <%this._tokenStartColumn===<i>%> -ImportListener(X) ::= <<var <X>Listener = require('./<X>Listener').<X>Listener;>> +ImportListener(X) ::= "" @ericvergnaud if I recall correctly these files are not needed anymore since we dropped the browser test suite in https://github.com/antlr/antlr4/pull/2755/files — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[carocad]

Suggested change

npm install

I think this is not needed since the runtime doesn't have any production dependencies only for development.

[ericvergnaud]

want to give it a try? you might be right, but if you are the cost is negligible

[carocad]

but if you are the cost is negligible

yeah, fair point. Lets keep it as it is

[ericvergnaud]

@parrt blessed

[ericvergnaud]

@ewanmellor
hi,
JS runtime is upgraded to ES6, which requires node 14 and npm to run the tests
I haven't managed to get "npm link antlr4" to work on AppVeyor, so disabling node testing on AppVeyor for now
It's not blocking for releasing 4.9 since the tests pass on Linux, but if you want to have a go at it you're welcome