jperasmus / docsify-copy-code

A docsify plugin that copies Markdown code block to your clipboard
MIT License
99 stars 72 forks source link

Rollup (fixes #4) #5

Closed jhildenbiddle closed 6 years ago

jhildenbiddle commented 6 years ago

Hi again, @jperasmus

Hope you don't mind the proposed update. This addresses all of the items mentioned in #4.

FWIW, I'm a day or so away from releasing a theme system for docsify, and in preparing for the release I've been testing all of the docsify plugins to see how I can provide theming options for them. While doing so I've stumbled on issues with a handful of plugins and have been offering PRs to help out. This latest PR is a substantial change, but it essentially just brings docsify-copy-code up to speed with best practices that other plugins are following.

Happy to discuss any of the proposed changes, and thanks in advance for your consideration. 😄

jhildenbiddle commented 6 years ago

One more idea to run by you...

To ease the transition from v1.x v2.x, we can create a placeholder window.DocsifyCopyCodePlugin.init function that simply prints a deprecation warning to the console. This way users who have linked to the latest version on unpkg.com won't see an error in the console about window.DocsifyCopyCodePlugin.init not being defined.

jhildenbiddle commented 6 years ago

Deprecation warnings added for calls to init() and the presence of a link to the v1.x stylesheet. 😄

jperasmus commented 6 years ago

I am not particularly opposed to the idea of using Rollup, I am just not that familiar with it. Personally using Webpack for all my personal and professional projects. Want to run this locally to play with it a bit and will then feedback on the proposed changes.

Your theming solution sounds very promising, please share a bit more once it is ready. Would love to check it out.

jperasmus commented 6 years ago

Hi @jhildenbiddle

I've had some time now to look through the changes you proposed. This is great work, thanks for it. Can you please add the dist directory to the .gitignore file, so that the generated artefacts aren't added under version control? Once this has been done, I'll merge, bump the plugin version and publish to NPM.

jperasmus commented 6 years ago

Thanks, I see the dist files are still in the PR, but I'm going to merge. Will fix those when I bump the version.

jhildenbiddle commented 6 years ago

Fantastic!

I was half-way through a response detailing some of the advantages of rollup over webpack, although either one will do the job. For what it's worth, the general rule seems to be "rollup for distributable libraries, webpack for apps". Things have changed over time now that rollup offers code splitting (previously Webpack only) and Webpack offers tree-shaking (previously rollup only), but rollup still produces smaller bundles and allows outputting ES6 modules (something Webpack still can't to AFAIK).

All that being said, the tool is less important than simply 1) removing the need to manually initialize, 2) removing the external stylesheet requirement by bundling CSS in JS, 3) providing a minified version for CDNs, and 4) providing a UMD for those who may want to create their own bundle that includes docsify-copy-code.

Whew. 😄

I've updated the .gitignore file as requested. Happy to make any other changes, so just give me a shout.

Thanks again!!

jhildenbiddle commented 6 years ago

Ahh yes... sorry about that. Forgot to remove the dist files from the repo.

One more thing -- the CHANGELOG.md should either be updated (with dates and changes) or removed. Currently it's just a placeholder because I didn't know when/if this PR would get merged and a new version published.

jperasmus commented 6 years ago

Thanks for all the info. I'll have to play a bit more with Rollup to see the size difference of the bundle(s) it produces. I've been quite happy with Webpack and also use it for some other libraries. It can produce UMD output as well.

Thanks again for all your contributions, it has been a pleasure working with you.

jhildenbiddle commented 6 years ago

You as well, @jperasmus .

FWIW, if you prefer Webpack by all means make the switch (not that you need my persmission to do so -- it's your repo!). No offense will be taken on my end if this PR serves only as inspiration for the same results made with a different tool.

Thanks again, and expect an email from me shortly regarding the docsify theme system!

jperasmus commented 6 years ago

Cool, looking forward to that email 👍