code reworks, bitHound fixes, attached eslint

[Delagen]

No description provided.

[coveralls]

Coverage decreased (-0.2%) to 98.597% when pulling b0b12a0 on Delagen:master into 354aab9 on NaturalIntelligence:master.

[Delagen]

Don't merge it. It fails compatibility

[amitguptagwl]

@Delagen can you please provide more detail about the code changes, compatibility with browsers, IE check, performance check etc.?

[Delagen]

@amitguptagwl I simply make some code arrange and tries to solve lint issues bitHound found. I think to migrate to ES6+ codebase and make browser bundle using webpack or other bundler with transpilation )

[amitguptagwl]

Thanks @Delagen it is really helpful. Let me check changes separately for performance and browser compatibility before merging.

I tried ES6 migration a long time back, but it impacted performance so I reverted back my changes as it is limited to this project only. I'm not sure if I was doing anything wrong that time or if that degradation is still applicable on current code.

[Delagen]

@amitguptagwl Now it may be more working.
I make main lib/parser.js.
It works in older node.js but Travis install all dependencies and webpack cannot install due engine restriction.
Bundle after generation need some fix I specified in webpack.config.js.

[amitguptagwl]

Sorry for the late response, it's a festive season here. So I'm not getting the chance to check for performance and compatibility. will do it after Monday.

[amitguptagwl]

I've just run perf tests. It seems there is not impact due to this merge

[Delagen]

It does not affect performance. But allow to use all ES features like arrow functions, const/let, destructuring and others, and make ES5 compatible output

[amitguptagwl]

This observation may or may not impact. Need to check it

this.attrsMap = {}; // in xmlNode.js may break nimn parsing.

webpack should generate beautify file instead of minified as CDN is already taking care of that, or both minified and normal.

[Delagen]

@amitguptagwl mode: production enabled minify by default.

[Delagen]

@amitguptagwl I merged master but fails latest undefined check ((

[amitguptagwl]

I have also resolved conflicts in my local for your merge. But somehow not able to push the changes. :(

[Delagen]

Resolved error )

[amitguptagwl]

Thanks @Delagen . I should have kept patience.

[Delagen]

squashed history )
I prefer Typescript, but migration often need for code rewrite and some authors prefer to write JS. )
ES2015+ functions offer more clean code )

[amitguptagwl]

:)

@Delagen any benefit of

• removing LICENSE file?
• in package.json, pointing main to generated file instead of src/parser.js

[Delagen]

@amitguptagwl Sorry). Restored LICENSE
main pointed to generated to support old version of Node.JS and browsers )
I removed testing from travis versions older than 6 cause it fail only for cannot install webpack due engine. But must work when from NPM

[amitguptagwl]

Hmm, thanks. Can it impact the way it includes in other npm packages? I hope you must have already tested it. But just wanted to double check.

[Delagen]

@amitguptagwl It's must work. But I didn't make deep tests. It must works also on IE 9 and later.
On IE7+ works with a little polyfill for defineProperty.
At least browser tests works in zombie. For multibrowser testing need to add some service which as I know not free )
One note for due bug in webpack webpack/webpack#6522, need replace window with this||window in output bundle.
I think to later move to mocha & puppeteer(or karma) for browser testing