[ES] migrate to mocha & nyc, ES code refactor, split bundle

[Delagen]

No description provided.

[Delagen]

Don't merge it until I fully test it

[amitguptagwl]

@Delagen Many thanks for your contribution to keep this repository up to date. I would appreciate if you can please mention the purpose/goal along with what is being changed. E.g. the title of this PR says that you are attempting to replace jasmine tests with mocha but it is not mentioned in the description how it can help in regards of this work-space.

[Delagen]

Jasmine or mocha seems no too much difference. Main goal of this is to offer 2 bundles: for node.js with cli, and for browser. So no need to manually edit code after build. And replace some classes from prototype declaration to class, some functions to arrow, some loops to for-of to reduce code, move exports to bottom to make it single place. Turn webpack to development for node bundles for code readability, provide sourcemaps.
But coverage decreased due ES6 features like it doesn't hit loop declaration but hit loop body )

[amitguptagwl]

Can we separate change which are not related to each other? E.g. mocha in this case.
Till now, cli code was not being merged to bundle. Is it happening after the last code refactoring when we replaced browerify with webpack?

[Delagen]

@amitguptagwl cli code now at src folder, and bundle to parser.cli.js. Mocha and nyc are very small changes, so it commonly in package.json, mocha.opts, .nycrc, .travis.yml changes

[amitguptagwl]

It's my mistake that I missed to notice that cli file has been moved. However keeping it in src folder is not a wrong decision. But it was not expected to combine with bundle file in last change. So it was a miss. apologies, I should have tested that.

We (including me) are having the habit of combining small change in single commit/PR. But if we have to revert some partial change then it become difficult to analysis, and to revert as well. So if the changes are not related they should not be combined.

[Delagen]

@amitguptagwl This a large refactor of code, but it does not change logic at all. I commonly code reorganize. But I understand that need to split commits.
I still cannot understand why coverage fallen. Seems it simple some incompatibility with ES code.

[Delagen]

Strange. Coveralls does not update status. But in it's interface all fine

[Delagen]

Strange. You reverted to browserify to preserve bundling in travis.
I think it can bundled by developer, and only tested in travis without some devDependencies

[amitguptagwl]

You reverted to browserify to preserve bundling in travis.

No. Please read the git commit message.

Previous commit was having some issues with webpack (as we discussed in previous comments) and I didn't want to keep the workspace in the state where any new pull request or any new change are required to be kept on hold.

As I said before, we should refrain to switch to new technologies / packages / libraries / concepts until they are giving any project specific advantage. I don't see any advantage of using webpack over browserify in this case. Hence I decided to revert it back until we find the better use case.

[Delagen]

@amitguptagwl
webpack only needed to bundle code and needed only for developer. This does not affect any of targets of deploy. It may be any ES5 compatible target. I use webpack for develop with typescript code that executed even in IE7 with small polyfill with 2Kb.
Browserify is difficult to config, even with small changes. It to old tool for today use.

[amitguptagwl]

Ah! I'm sorry I couldn't explain my point very well. It was not about how it is impacting / affecting, but about how is it beneficial. So if I ask, e.g., what will be changed when we'll move from browerify to webpack in the current scenario?

[Delagen]

@amitguptagwl For example now, code failed with

Const must be initialized

on IE 11.
Seems babelify transform need to be added.
But babelify fail integration with latest babel babel/babelify#255, so it only works with 6.x )

[amitguptagwl]

it's a good point, let me check

[Delagen]

@amitguptagwl
Again broken IE compatibility

  tagValueProcessor: a => a,
    attrValueProcessor: a => a

[amitguptagwl]

fixed

[amitguptagwl]

We're extremely sorry to close this PR as it is open from a long time and containing multiple changes in a single pull request.

[Delagen]

@amitguptagwl this changes align code to ES6. But I think it not needed as it not merged for such a long time )

[amitguptagwl]

@Delagen As I conveyed earlier single PR should ideally contains single change. It contains more than ES6 changes.