Javascript: migrate prototypical root modules to ES6

[carocad]

This PR is a follow up on #2771.

The scope of this PR is to refactor the root js module declarations to:

• use es6 classes/inheritance instead of direct prototype manipulation.
• refactor exports to directly expose a single class whenever possible
• use const/let on variable declarations to improve variable scoping
• refactor comments to be JsDoc compatible which most IDEs can leverage to provide better developer feedback

Implementation notes:

• due to the refactor from prototype to class inheritance, it was not possible to completely avoid chaging the JavaScript templates. This is because ES6 forbids the use of call method on classes
• I tried to avoid mofiying too much of the templates in order to reduce the scope of this PR as much as possible

[ericvergnaud]

@parrt blessed

[ericvergnaud]

Hey, quite a massive investment!
This is going to make our lives much better :-)
Am I correct in thinking we're done with the proto->ES6 conversion?
If you still have some energy left, I'd suggest the following improvements:

• 1 file per ES6 class (à la Java)
• import instead of require
  Let me know if you have other improvements in mind

[carocad]

Am I correct in thinking we're done with the proto->ES6 conversion?

Not entirely. The templates still modify the prototype directly. As mentioned in the PR description I tried to avoid changing too much at the same time so I changed the function <name> ... to be class <name> extends <super> ... but the rest is still the same.

I guess that would be for another PR 😅

import instead of require

I would not aim for this right now since import style is experimental in nodejs. This means that you would need a transpiler for using the code even in node.js which I think defeats the purpose.

[ericvergnaud]

Am I correct in thinking we're done with the proto->ES6 conversion?

Not entirely. The templates still modify the prototype directly. As mentioned in the PR description I tried to avoid changing too much at the same time so I changed the function <name> ... to be class <name> extends <super> ... but the rest is still the same.

I guess that would be for another PR 😅

import instead of require

I would not aim for this right now since import style is experimental in nodejs. This means that you would need a transpiler for using the code even in node.js which I think defeats the purpose.

Fair enough, I've been using it with node for a few years now, and stopped noticing the above because I need a compiler for JSX anyway...

[piotrl]

I step by to say a word of appreciation, this investment is huge step forward for JavaScript target! Making it easier to use antlr4 with modern front-end toolbox out of the box.

So far we needed to maintain our own pre-compiled fork of antlr4 for web purposes, with those changes + published antlr4 for web package, we'd be able to drop maintenance of the fork :)

[carocad]

@piotrl thanks for that. It doesnt happen often that someone stops by to say thanks on an open source project 😃 .

So far we needed to maintain our own pre-compiled fork of antlr4 for web purposes, with those changes + published antlr4 for web package, we'd be able to drop maintenance of the fork :)

Why not take a step forward and help make it even better 😉 . I am pretty sure there is lots of thigns that you could contribute to the project.

On another topic ...

Not entirely. The templates still modify the prototype directly. As mentioned in the PR description I tried to avoid changing too much at the same time so I changed the function ... to be class extends ... but the rest is still the same.

@ericvergnaud I have been trying to get this done but so far I have failed miserably. The problem that I am facing is that the template seems to assume that it is possible to have nested classes, which is not in Javascript. That means that for example here, it is not possible to "finish" the class declaration because another class starts in ruleCtx. Therefore a prototype modification is required. I dont really get how the templates work so if you can give me some guidance there it would be much appreciated :)

[ericvergnaud]

I’m a bit busy for the next 2 weeks, maybe shout back then? I’ll look into it.

…

Le 17 avr. 2020 à 23:13, Camilo Roca ***@***.***> a écrit : @piotrl <https://github.com/piotrl> thanks for that. It doesnt happen often that someone stops by to say thanks on an open source project 😃 . So far we needed to maintain our own pre-compiled fork of antlr4 for web purposes, with those changes + published antlr4 for web package, we'd be able to drop maintenance of the fork :) Why not take a step forward and help make it even better 😉 . I am pretty sure there is lots of thigns that you could contribute to the project. On another topic ... Not entirely. The templates still modify the prototype directly. As mentioned in the PR description I tried to avoid changing too much at the same time so I changed the function ... to be class extends ... but the rest is still the same. @ericvergnaud <https://github.com/ericvergnaud> I have been trying to get this done but so far I have failed miserably. The problem that I am facing is that the template seems to assume that it is possible to have nested classes, which is not in Javascript. That means that for example here <https://github.com/antlr/antlr4/blob/master/tool/resources/org/antlr/v4/tool/templates/codegen/JavaScript/JavaScript.stg#L245>, it is not possible to "finish" the class declaration because another class starts in ruleCtx. Therefore a prototype modification is required. I dont really get how the templates work so if you can give me some guidance there it would be much appreciated :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2787 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZNQJDVAIEBPTE53LHZA3LRNBWYDANCNFSM4LROVWYA>.