Don't bundle

[jaydenseric]

The main package entry points to lib/react-rating.js, which is a huge Webpack bundle.

Bundling is something consumers should do once for their entire app, packages published to npm should not be bundled or minified. Dependencies such as prop-types are being copy-pasted into this package, instead of being a proper declared dependency and using require:

object-assign is a React dependency, so by bundling it in this package, a consumer gets the React one and your one bundled into their app, instead of sharing one node_modules dependency:

You can actually configure Babel in a few places to use native Object.assign instead of helpers to reduce bundle impact, then for cross-browser support use babel-plugin-transform-replace-object-assign to transpile all the Object.assign instances to use the same object-assign dependency React uses. Here is an example: https://github.com/jaydenseric/graphql-react/blob/master/babel.config.js

You can then use size-limit to test bundle impact, ignoring object-assign since you know it is already in React projects anyway: https://github.com/jaydenseric/graphql-react/blob/master/package.json#L115

I can think of other optimizations, but those are some low hanging fruit. Just dump Webpack and use the Babel CLI directly. You should be able to at least half the bundle impact for consumers.

[jaydenseric]

Pre-bundling packages also means analysis tools can't determine the dependency graph properly.

Compare:

https://bundlephobia.com/result?p=react-rating@1.4.0

With:

https://bundlephobia.com/result?p=graphql-react@2.0.0

[dreyescat]

I remember someone requesting to publish the src folder to npm just to have more control on how to bundle this component (#40). Let's do it well.

I am not pretty aware of the current right way for publishing a component like this 😅. What would be better, pointing the main field to the non-transpiled version of the library (let's say the ES6 version) or a transpiled version but not bundled one? Would you keep the bundled version?

I have checked your project's package.json and I have even seen you use the module field (just googled to check what it means). I would like you to give me advise. Or even better, if you could create a PR I will be pleased to accept it.

[jaydenseric]

Don't do any bundling or minification when publishing to npm.

Always publish transpiled code though, preferably using @babel/preset-env configured via a browserslist query in package.json for your chosen level of browser support.

Publish two builds:

1. main entry should point to a CJS build.
2. module entry should point to an ESM build.

Both should be transpiled the same except for the module format.

The ESM allows consumers installing your package to do better tree-shaking and other optimizations via Webpack when compiling their project.

Bonus points is to make sure your ESM build puts out .mjs files, which are suitable to run in Node.js in --experimental-modules mode. This is a little more advanced to get right; the files should be sibling to the .js files and all local import paths should be extensionless, including the path in the package main field (e.g. instead of "lib/index.js" use "lib/index", or just "lib"). Node.js will resolve the .mjs files in preference to .js in --experimental-modules mode. If you do this right technically you can drop the module field as Webpack will automatically prefer the .mjs files too. If you try to support native ESM via .mjs be sure to actually try running it before publishing to see that it works!

Sorry I won't be able to work on a PR, I decided not to use this package in my project in the end and don't have spare time to contribute. I just left this issue to help out. Happy to answer any questions though :)

[carpben]

Not sure if this is the proper place.
I'm using the library, and it weighs 24kb in build. I'd be happy if it weighed less.

Though I'm not an expert I think it might be related to the build process, since if you bundle it can't share dependencies.

[dreyescat]

@carpben Yes. It is a bundling issue. We are bundling unnecessary stuff that increases the bundle weight.

We are still using the original build process when prop-types was not a separated package but part of the main react package 😅 and we were just worried about the umd format that works for all modules.

I would like to change this. Following @jaydenseric recommendation, the idea is to have different builds for umd, cjs, and esm. I have started to think about it and I am evaluating some options:

1. Rethink current Webpack configuration
2. neutrinojs
3. nwb
4. Rollupjs

[dreyescat]

@carpben After some reading I decided to give Rollupjs a try. It easily allows us to produce different package formats.

Packages size has been reduced.

I have just published a beta version with the changes available at npm:

npm install react-rating@beta

Let me know if it helps.

[carpben]

@dreyescat, Looking good.
I ran:

npm uninstall react-rating
npm install react-rating@beta 

It worked with the same functionality. size of library in build decreased significantly - from 23.5kb to 8kb.

For some reason in my package.json it showed the version as 1.71. Perhaps you haven't changed the version number in the library's package.json?

[dreyescat]

Cool! Thanks @carpben.

Yes. I saw an important size reduction. I wanted you to confirm. I have also removed the proptypes. Maybe there is still room for improvement because I still see some generated code in the compiled files, but I do not expect such a reduction as the one we have accomplished now.

Later I will remove the beta tag and make it available as latest version.

For some reason in my package.json it showed the version as 1.71. Perhaps you haven't changed the version number in the library's package.json?

That's Ok. 1.7.1 is going to be the next version. The only thing I did is not to publish it as latest but as beta to prevent others to accidentaly get this version before we are sure it works 😉. I mean, if someone runs npm install react-rating the 1.6.2 version is installed.