Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use uglifyjs CLI instead of Gulp to minify #1046

Merged
merged 4 commits into from Feb 9, 2018

Conversation

Feder1co5oave
Copy link
Contributor

@Feder1co5oave Feder1co5oave commented Feb 9, 2018

Following from #1041.
By dropping gulp we get rid of a lot on unused dependencies.
Uglify-js3 now takes care of minification. A shortcut to minify is npm run build.

Make sure to npm prune && npm install after pulling.

If you want to make sure that the minified version behaves as expected, run node test --minified to test it.

@joshbruce
Copy link
Member

@Feder1co5oave:

Can you expand on this bit??

Make sure to npm prune && npm update after pulling.

@Feder1co5oave
Copy link
Contributor Author

To update the dependencies, as you would on a fresh checkout. Pruning is not mandatory, it gets rid of everything related to gulp. You can even rm -r node_modules and npm install.

@Feder1co5oave Feder1co5oave mentioned this pull request Feb 9, 2018
7 tasks
@joshbruce
Copy link
Member

Okay, just checking to make sure. Never saw the prune command before; thanks for the introduction.

Given this would help reduce the overall size of the installation footprint, I'm thinking we'll go ahead and merge this. Maybe do the lint as well. Then publish despite not strictly part of the 0.4.0 goal (don't want to lose too much of the consistent flow of updates we've had (seems like people are taking notice that the project is alive again). Thoughts??

Tagging @UziTech as well.

@UziTech
Copy link
Member

UziTech commented Feb 9, 2018

I think this is a step in the right direction but I would ultimately like to start using ES6+ to modularize the codebase so it is cleaner. At that point we will have to replace this with babel or some other transpiler.

@joshbruce
Copy link
Member

@UziTech: Fair. Then let's go ahead and merge this in as that step. Maybe start an Issue on the ES6 question to aid the 0.5.0 milestone??

@joshbruce joshbruce merged commit d96db05 into markedjs:master Feb 9, 2018
@Feder1co5oave
Copy link
Contributor Author

There's #746 for discussing modularization! Let's move over there

@Feder1co5oave Feder1co5oave deleted the uglifyjs branch February 10, 2018 01:57
@Feder1co5oave
Copy link
Contributor Author

@joshbruce Wow, I wrote the following message right after your comment but totally forgot to send it out! 🤦‍♂️
Anyway, here it is for y'all to enjoy 😆


Actually I think a simple npm install takes care of everything inside node_modules.

Anyway, this PR, #1020 and #1019 take care of everything about dependencies we discussed in #1041. I wouldn't bother publishing for just these ones, but we have 62 commits since v0.3.12 and we fixed quite a lot of issues!

I terms of commonmark compliance, we fixed

  • link definitions
  • blockquotes
  • autolinks (gfm fashion too)
  • thematic breaks

Also from the user perspective we "fixed" the "replace is not a function" stupid error message so hopefully we won't get any new issue opened because of that.
I'm currently working on fixing links (so #984 here I come) but that could take a while, and also some other things for the future.
Your choice to release or wait. Anyhow it's good to send out the message that we're alive and fighting.

zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
Use uglifyjs CLI instead of Gulp to minify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants