turbolent / collection-view

UICollectionView for the web
MIT License
66 stars 7 forks source link

Unminify Build #39

Open jsmith opened 3 years ago

jsmith commented 3 years ago

Hey, thanks for this great library! I'm just starting to use it for a side project of mine and noticed that the build is minified. This is great for production but is not ideal for development. Since my build pipeline already does minification (I expect this is the case for most people using the library), would you be interested in using something like rollup to build the library for distribution? If so, I don't mind making this change and creating a PR :)

jsmith commented 3 years ago

Also I realized that the package.json module field points to src/index.js which doesn't exist in the npm package. The rollup PR could also this issue!

jsmith commented 3 years ago

Ok I unfortunately ran into issues where the grid elements weren’t being displayed past index ~ 50 in a two column grid but I had 1000 elements (I was running a modified react example). I tried to debug but wasn’t that successful so I’m going to look at alternative packages (which makes me a bit sad since the animations were sooo smooth). Thanks again!

turbolent commented 3 years ago

@jsmith

Since my build pipeline already does minification (I expect this is the case for most people using the library), would you be interested in using something like rollup to build the library for distribution? If so, I don't mind making this change and creating a PR :)

Right, I think the optimization in the Webpack configuration should be disabled, given this is a library: https://github.com/turbolent/collection-view/blob/master/webpack.config.js#L37-L42.

I'm mostly using Webpack here because I want to compile the TypeScript code, want to embed the CSS, and because I'm somewhat familiar with it.

A PR would be very welcome, I think I can also easily do this.

Also I realized that the package.json module field points to src/index.js which doesn't exist in the npm package. The rollup PR could also this issue!

Good catch! The output is in dist, so the package file should point to dist/index.js.

unfortunately ran into issues where the grid elements weren’t being displayed past index ~ 50 in a two column grid but I had 1000 elements (I was running a modified react example).

Oh, that should not happen! Would you be able to provide code to reproduce it? Also, do you have more details on your environment, e.g. what browser(s)? I might find time to look into this.

jsmith commented 3 years ago

Right, I think the optimization in the Webpack configuration should be disabled, given this is a library: https://github.com/turbolent/collection-view/blob/master/webpack.config.js#L37-L42.

I think disabling that for during the build would be great! Since it's already working with webpack, there's probably no need to refactor to rollup. When I made this issue, I hadn't realized you were bundling up the CSS too. I'm honestly not sure what webpack does with the CSS and not sure how easy it would be to replicate that with rollup.

Good catch! The output is in dist, so the package file should point to dist/index.js.

I think the module field could actually be removed since the build only produces a single CommonJS output file and the main field already points to this file. Also, the same issue happens in heckel-diff-items. To get both libraries working in my project for testing, I just removed the module field!

Oh, that should not happen! Would you be able to provide code to reproduce it? Also, do you have more details on your environment, e.g. what browser(s)? I might find time to look into this.

I uhhh couldn't reproduce this issue, which I guess is a good thing! My reproduction codesandbox works great with two columns :)