issue/214-es6-upgrade

[littlespex]

This PR updates the code base and build tools to ES6. It replaces jshint, browserify and closure compiler with aslant, rollup, terser and typescript (only for ES5 compilation and basic type declarations, the source files are still JS). If a full typescript conversion is desired, I can piggy back that change onto this PR.

[littlespex]

@palemieux Here is base ES6 conversion. Let me know if you'd be interested in converting to TypeScript.

[palemieux]

Here is base ES6 conversion. Let me know if you'd be interested in converting to TypeScript.

Thanks @littlespex ! Will review.

[littlespex]

Fixed the bad import and reformatted with the lint changes.

[palemieux]

@littlespex Still seeing:

>> [!] RollupError: dist/main/styles.js (27:37): "parseInt" is not exported by "dist/main/utils.js", imported by "dist/main/styles.js".

Do you know why I would be seeing it and not you?

[littlespex]

@littlespex Still seeing:

>> [!] RollupError: dist/main/styles.js (27:37): "parseInt" is not exported by "dist/main/utils.js", imported by "dist/main/styles.js".

Do you know why I would be seeing it and not you?

Because I had local changes that weren't committed :)

[palemieux]

@littlespex An initial objective of imscJS was to allow generateISD() to be called from node since it did not contain any DOM dependencies. Would anything in the refactoring prevent this?

[littlespex]

The only difference is that it would need to be imported now instead of required. I can update the build step to generate a cjs version of the library as well if you want to keep a "requirable" version of the lib.

[palemieux]

I can update the build step to generate a cjs version of the library as well if you want to keep a "requirable" version of the lib.

Let's definitely not support "require" :)

[palemieux]

@littlespex I am thinking we could create a 1.2 branch using yours as a basis... we might have other improvements, e.g., factoring out XML parsing.

[littlespex]

@palemieux I've added a GitHub action to run CI (lint, build, and test) for every pull request. The unit tests in src/test/js and src/test/webapp/js/unit-test.js have been converted to use node's built in test runner. The original tests have been left in place, but doesn't appear that any of them actually need to be run in a browser environment. Can we remove them in favor of the new ones?

[palemieux]

Can we remove them in favor of the new ones?

+1

[palemieux]

@littlespex Ok to branch or do you have more improvements to make re: conversion to ES6 + modern tooling

[littlespex]

I don't have anything else.

[palemieux]

@littlespex One last request/question: can we move the ./test/*.js under ./src/test/js and ./test/resources/* under ./src/test/resources/?

[littlespex]

Done

[palemieux]

>> dist/main/main.js → dist/imsc.all.debug.js, dist/imsc.all.min.js...
>> (!) Circular dependencies
>> polyfill-node._stream_duplex.js -> polyfill-node._stream_readable.js -> polyfill-node._stream_duplex.js
>> polyfill-node._stream_duplex.js -> polyfill-node._stream_writable.js -> polyfill-node._stream_duplex.js
>> created dist/imsc.all.debug.js, dist/imsc.all.min.js in 1s

Is this an issue?

[littlespex]

No. It's just a warning when rollup bundles the sax package. It resolves these correctly and bundles the "all" version of the library without error.

[palemieux]

Created https://github.com/sandflow/imscJS/tree/integration/v2 and https://github.com/sandflow/imscJS/milestone/12

Thanks!