antlr4ts
antlr4ts copied to clipboard
Add browser support via babel
This takes a different approach from #413. Instead of modifying dozens of typescript files in the fragile hopes of getting it to compile nicely to es5, I took the es2015 generated by TS and gave it to babel to transform. This seems simpler and easier because instead of relying on buggy TS es5 support, we can use industry standard babel which will hardly ever fail at transforming es2015+ to es5. I do not think the responsibility of getting working es5 should fall on the TS programmer. I used gulp to take the output of TS and stream it to babel without the need to hit the filesystem twice while compiling.
I had tried to use rollup before (#422) but rollup is not suited to compiling individual files to es5. It is a bundler and designed to output the whole library into a single file. This breaks clients which import/require the individual files such as "antlr4ts/atn/ActionTransition". Rollup would only help in making a bundle file meant to be added directly to client HTML to reduce script requests, perhaps as a UMD file. However, most client projects will have their own bundler that statically includes all necessary library code.
Thank you for the pull request. I'm looking forward to reviewing it.
You touch on a point I think may have contributed to the stalling of this project. We did work a few years ago that should (if implemented correctly) eliminate the need for import statements that delve into the directory structure like
import { ActionTransition } from "antlr4ts/atn/ActionTransition";
can and should be eliminated. The Façade Pattern implemented in the index.ts files means that
import { ActionTransition } from "antlr4ts";
should work, and is less fragile. Of course, as you correctly state, that would be a breaking change, but that's what major versions in semantic versioning are for. We're well overdue for a V1 release, and I think some cleanup of the way clients reach into the run-time code is would be appropriate. Bundlers like rollup and webpack are an important part of the modern javascript ecosystem, but neither @sharwell nor I had any experience with them when we started the port.
Do you have any specific reason to say that Typescript's ES5 support is buggy?
The Façade Pattern implemented in the index.ts files means that
import { ActionTransition } from "antlr4ts";should work, and is less fragile.
On the one hand it would be better to be able to rearrange library directory structure without impacting clients in future versions, on the other it forces clients to have to have a tree shaking bundler if they want to minimize their browser bundles. It is not my call to make.
Do you have any specific reason to say that Typescript's ES5 support is buggy?
If you enable these options in tsconfig
"lib": [
"ES2017",
"ES2016",
"ES2015",
],
"downlevelIteration": true,
"target": "ES5",
you will quickly run into problems #413 was facing, first of which is:
error TS2340: Only public and protected methods of the base class are accessible via the 'super' keyword
Looking at TS's issue you will see that they don't really care to support this compilation to ES5. That is a specific example but in general it is very telling of their support for ES5 compilation. Even though babel can compile the code just fine from ES6 to ES5, Typescript forces you to think about ES5 compilation quirks even with valid Typescript. If TS only fully supports compilation to ES6 then we should use babel to compile to ES5. There are many more of these sorts of problems that were addressed in #413.
One perk of using babel is that with @babel/preset-env you can choose the coverage of browsers to support in ES5 via a browserlist query
@unsupervisednn @BurtHarris, it looks like the build is failing because the CI uses Node.js v6.
Specifically, gulp-typescript relies on source-map which requires Node.js 8+. (gulp-typescript#584, permalink to engines)
The README.md points to supporting Node.js 6.7.x.
AppVeyor step that downgraded to v6 First ReferenceError regarding WebAssembly
What's blocking this from being merged?
Any updates on this?
I took a shot at running this code in the browser, but unfortunately ran into errors with various Nodejs modules that are needed, like util and assert. There are polyfills for some of them, but they are different for eg Webpack and Vite, so it might be good to move away from them overall.
Using Rollup I was able to get this to work in the browser, shimming util and assert. Would be nice to have this merged in.