Skip to content

Commit

Permalink
feat(browser): added browserify UMD tasks to the build, still need to…
Browse files Browse the repository at this point in the history 
… update the STG template
  • Loading branch information
@petermetz
petermetz committed Jul 21, 2017
1 parent 9db6b7e commit c058037
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
7 changes: 7 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
"antlr4ts-benchmark-lr": "cd benchmark && antlr4ts -visitor JavaLR.g4 -DbaseImportPath=../../../src -o gen/lr",
"antlr4ts-benchmark-lr-atn": "cd benchmark && antlr4ts -visitor -Xforce-atn JavaLR.g4 -DbaseImportPath=../../../src -o gen/lr-atn",
"tsc": "tsc",
"browserify": "./node_modules/.bin/browserify ./target/src/index.js --transform [ babelify --presets [ es2015 ] ] --debug --outfile target/antlr4ts.umd.js --standalone antlr4ts",
"browserify-min": "./node_modules/.bin/browserify ./target/src/index.js --transform [ rollupify --config rollup.config.js ] --transform [ babelify --presets [ es2015 ] ] --outfile target/antlr4ts.umd.tree-shaken.js --standalone antlr4ts",
"buildrts": "cd test/runtime && tsc",
"test": "mocha",
"cover": "istanbul cover node_modules/mocha/bin/_mocha",
Expand Down Expand Up @@ -52,9 +54,14 @@
"devDependencies": {
"@types/mocha": "^2.2.32",
"@types/node": "^6.0.41",
"babel-preset-es2015": "^6.24.1",
"babelify": "^7.3.0",

This comment has been minimized.

Sign in to view

Copy link
@BurtHarris

BurtHarris Jul 27, 2017

I'm really a bit gun shy about including Babel in this project. Isn't one transpiler (TypeScript) enough?

This comment has been minimized.

Sign in to view

Copy link
@petermetz

petermetz Jul 27, 2017

Author Owner

I couldn't agree more with that in principle, but practice showed that at the moment you cannot actually set the Typescript compilation target to es5 due to the usage of Maps/Sets (I think those were the ones) in the code.

These are not supported by Typescript for ES5. I read some very thoughtful explanation about the reasoning behind this on their issue tracker, which I did not fully understand to be honest, but the bottom line was if I got it right that they aren't planning on changing this because you should use Babel for these cases.

So what I managed to put together is that it's either a) Babel or b) we cannot use certain language features from the newer specs, despite the fact that Typescript should take care of that.

My vote goes for Babel because that can be just removed once it becomes pointless to support ES5 (that could be still a decade though...) while with the other option whenever we drop ES5 support, it's already too late because the code was written with old language features (e.g. no Map/Set, etc.).

This comment has been minimized.

Sign in to view

Copy link
@BurtHarris

BurtHarris Jul 27, 2017

@petermetz: Its a fine point, but worth understanding: the Map/Set support doesn't require --target es2015, it just requires --lib es2015 which is one way(of several possible) of getting the right declarations in scope. In theory Map/Set can be handled by a Polyfill, but that is untested in this codebase, primarily because Sam and I initially tried to limit the scope to modern versions of Node.js.

The reason I needed --target es2015 was something different having to do with iterators, see discussion in issue tunnelvisionlabs#311, it has pointers to the relevant TypeScript issue. The --target es2015 workaround should no longer needed be needed. Your review of (and possible PR for) tunnelvisionlabs#311 would be appreciated. I'd be very open to a PR that removed the --target es2015 (or equivialant in tsconfig.json.)

This comment has been minimized.

Sign in to view

Copy link
@petermetz

petermetz Jul 28, 2017

Author Owner

@BurtHarris Hmm, didn't think of using polyfills for some reason. My bad! I'll give this one a go in the browser and if it works I'll dump Babel immediately. :-)
"browserify": "^14.4.0",
"istanbul": "^0.4.5",
"mocha": "^3.1.0",
"mocha-typescript": "^1.0.10",
"rollup-plugin-babel": "^2.7.1",
"rollupify": "^0.4.0",
"source-map-support": "^0.4.3",
"std-mocks": "^1.0.1",
"typedoc": "^0.5.1",
Expand Down
7 changes: 7 additions & 0 deletions rollup.config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
plugins: [
require('rollup-plugin-babel')({
exclude: 'node_modules/**'
})
]
}

This comment has been minimized.

Sign in to view

Copy link
@BurtHarris

BurtHarris Jul 27, 2017

Please make sure to end your files with a newline

This comment has been minimized.

Sign in to view

Copy link
@petermetz

petermetz Jul 27, 2017

Author Owner

Whoops, thanks for the note, will definitely do!
11 changes: 8 additions & 3 deletions src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

export * from './ANTLRErrorListener';
export * from './ANTLRErrorStrategy';
// export * from './ANTLRFileStream';
export * from './ANTLRInputStream';
export * from './BailErrorStrategy';
export * from './BufferedTokenStream';
Expand Down Expand Up @@ -43,8 +42,14 @@ export * from './TokenFactory';
export * from './TokenSource';
export * from './TokenStream';
export * from './TokenStreamRewriter';
// export * from './UnbufferedCharStream';
// export * from './UnbufferedTokenStream';
export * from './Vocabulary';
export * from './VocabularyImpl';
export * from './WritableToken';

export * from './misc/Utils';
export { NotNull, Override } from './Decorators';
export * from './index';
export * from './atn/index';
export * from './dfa/index';
export * from './misc/index';
export * from './tree/index';

This comment has been minimized.

Sign in to view

Copy link
@BurtHarris

BurtHarris Jul 27, 2017

See/contribute to discussion on tunnelvisionlabs#340

1 comment on commit c058037

@BurtHarris
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments

Please sign in to comment.