fixes from #2 and #5

[cypherean]

Committing after running npm run build. Updates the dist/ folder and minor fix to heading test.

[cypherean]

Once this, and #11 (comment) is done, let's aim for integration into PLE and then we can add tests.

[jywarren]

Hi, would you mind rebasing? And just to check, this should be merged before #17 right? Just checking and cc'ing @Shulammite-Aso. Thanks!!!

[cypherean]

I rebased and manually fixed the conflicts for src/ files. On running npm run build to resolve conflicts of dist/ files I'm facing this error

/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/tools/node.js:42
    sys.error("WARN: " + txt);
        ^

TypeError: sys.error is not a function
    at Function.UglifyJS.AST_Node.warn_function (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/tools/node.js:42:9)
    at Function.AST_Node.warn (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/ast.js:110:18)
    at Compressor.warn (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/compress.js:87:27)
    at TreeTransformer.before (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/compress.js:1063:48)
    at AST_Function.transform (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/transform.js:61:35)
    at AST_Function.drop_unused (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/compress.js:1171:18)
    at Compressor.before (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/compress.js:99:18)
    at AST_Function.transform (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/transform.js:61:35)
    at /home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/transform.js:80:25
    at doit (/home/cypher/Documents/dev/outreachy/woofmark/node_modules/uglify-js/lib/utils.js:121:23)

which has mainly to do with uglify-js. I ran npm install as well but the error persists. Any idea what's wrong? @NitinBhasneria @Shulammite-Aso @keshav234156

[Shulammite-Aso]

@Shreyaa-sharmaa will things still work fine if you remove the dist files? I mean, not stage them at all?

[cypherean]

The dist/ files have merge conflicts. The main reason for this PR was to update and commit the dist/ files of the changes made.
@jywarren @sagarpreet-chadha do we need the dist files to be changed? And out of curiosity, how does an updated or not updated dist/ file affect the project that it is a dependency in?

[sagarpreet-chadha]

Hi @Shreyaa-sharmaa ,
Ideally we should not have dist folder commited in github repo as it is hard to track.
While publishing to npm, dist folder should be generated by publisher. Otherwise there is no point to keep dist folder.
However, as we are keeping dist folder in this repo...so the trick is to remove the dist file content and run the grunt command again to generate them fresh.

[jywarren]

Thanks for your patience on this. @sagarpreet-chadha is very correct. We've traditionally skipped this approach and i think it causes a lot of issues. But if we can clearly document the npmjs publication steps, either by writing in the README or in an NPM task, OR (ideally) both -- it'd help us be really consistent on this.

Thank you so much!

In the short term i'm sorry but if you could rebuild and rebase one more time, that would be great. 😭

[cypherean]

A little progress on this one, uglify-js doesn't support ES6, never has. Now this leads to errors for using const and let. Adding /* jshint esnext: true */ on top of the JS file (workaround found in a github thread) bypassed those errors but received another one for unrecognised token. I believe that we should look at other options with ES6 support as a more permanent option. What do you all think? Terser is a possible replacement. Quoting terser's readme:

uglify-es is no longer maintained and uglify-js does not support ES6+.
terser is a fork of uglify-es that mostly retains API and CLI compatibility with uglify-es and uglify-js@3 

Also, npm run build gives a few errors in the tests as Nitin mentioned in #11 (comment) please have a look.

[jywarren]

It sounds good to me! @sagarpreet-chadha, you've encountered issues like this before I think. What do you think?

…

On Wed, Jul 22, 2020 at 11:34 AM Shreya Sharma ***@***.***> wrote: A little progress on this one, uglify-js doesn't support ES6, never has. Now this leads to errors for using const and let. Adding /* jshint esnext: true */ on top of the JS file (workaround found in a github thread) bypassed those errors but received another one for unrecognised token. I believe that we should look at other options with ES6 support as a more permanent option. What do you all think? Terser is a possible replacement. Quoting terser's readme: terser is a fork of uglify-es that mostly retains API and CLI compatibility with uglify-es and ***@***.*** ``` Also, `npm run build` gives a few errors in the tests as Nitin mentioned in #11 (comment) please have a look. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J3LDR43TFLOPPOQJO3R44AUFANCNFSM4OZMAWWQ> .

[sagarpreet-chadha]

Oh okay, i think we should have es6 support in this library so that new JS developers can also contribute in this library, so yes we should remove uglifier js and use any new compressing package which supports es6 and is easy to use. Thanks!
Let me know if you need any help in setting up the new compressing package.

[cypherean]

Closing this one.