jofftiquez / vue-croppie

Vue wrapper for croppie
https://jofftiquez.github.io/vue-croppie/
MIT License
260 stars 42 forks source link

Improve build by swapping Webpack and exposing a real ESmodule #52

Closed dobromir-hristov closed 5 years ago

dobromir-hristov commented 5 years ago

So using Webpack to bundle a library is simply wrong, especially for ESM modules. Do you mind if I submit a PR changing webpack for Rollup or Bili (which uses rollup under the hood)? I will try to keep it without breaking changes.

I am coming from the fact that Croppie got a minor version bump, that fixes the enforceBoundary set to false issues. But because even the ESM module has Croppie bundled inside it, I cant update just the indirect dependency, without waiting for a release bump on behalf of vue-croppie. Not to mention the shit tone of extra code webpack adds, that is totally unneeded.

jofftiquez commented 5 years ago

Hi @dobromir-hristov

using Webpack to bundle a library is simply wrong

I totally agree. Kindly submit a PR, it will make this plugin better. Thank you :)

dobromir-hristov commented 5 years ago

One issue I came up to during setting up the build process is the inlining of css.

Usually its not a good idea to inline css from a library, it should be imported separately. I tried keeping things as backwards compatible as possible, but that would require for the ES and CJS builds to also inline Croppie, which is not how it should be. I am trying to figure it out so I will keep you updated.

jofftiquez commented 5 years ago

Great! Thank you so much @dobromir-hristov

dobromir-hristov commented 5 years ago

So I cant figure out a neat way to do this. Chose between the two:

  1. We release it the same way as before, with Croppie.js and Croppie.css inlined inside Vue-Croppie, making the package bigger and potentially forcing you to re-release a new version each time Croppie releases.
  2. We release it properly, with ES and CJS modules having CSS and Croppie imported separately (not inlined) and UMD module (for direct browser consumation) having those two inlined. This means a breaking change, as now users MUST import Croppie.css explicitly in their application. The benefit is that if Croppie release a new minor version, users can just npm update and Vue-croppie doesnt need to release. Also CSS will be handled by the users pipeline, so it can be extracted or however they need it to be.
jofftiquez commented 5 years ago

@dobromir-hristov Thank you for spending time on this. I think option number 2 is better. Importing an extra CSS wouldn't hurt the user's experience of using this plugin I think. So do you have any POC for this option?

dobromir-hristov commented 5 years ago

It wont, but its a breaking change. Everyone using upto now will have to explicitly import the css. The rest stays the same. And yes, I have a working example. I can push it a bit later. Do you want to have both the component exported, along with the default export? That way people can register croppie locally, instead of having it globally registered.

jofftiquez commented 5 years ago

Registering it locally would be much neater IMO. Thanks @dobromir-hristov

dobromir-hristov commented 5 years ago

OK then. Pushed and ready for review