search

Iconscout / react-unicons

1000+ vector icons as easy to use React Components
https://iconscout.com/unicons
Other
334 stars 39 forks source link

Add commonJS #9

Closed aravindballa closed 4 years ago

aravindballa commented 4 years ago

We need commonjs version of the library for a few use cases (like #2) of this package. We already ship this as ES6 modules, which is the best way when we want it to used in node as well as browser.

When targeting both browsers and Node.js, it's best to write our code using ES6 modules. Then with our build tool, we can create one CommonJS build and another UMD build. This way those using Node.js can import the CommonJS build. Browser users can either use the ES6 module build if using a bundler or the UMD build otherwise. Source

So I added webpack and spun up a small config file which gives us a commonJS version of the package. (Relevant Webpack docs)

The changes in the package.json are also picked from Webpack's suggestion in the docs. I also saw this pattern in another icon library called react-icons.

I could test this in NextJS and it seems to work. I created a new nextjs app, copied the changed unicons package to node_modules and added an icon in the JS file.

pavanmehta91 commented 4 years ago

Looks pretty good to me. Can you add the lib folder to .gitignore?

ivanahuckova commented 4 years ago

I have tested the solution and it worked in our repo as well. Thanks @aravindballa! ❤️

aravindballa commented 4 years ago

Added it @pavanmehta91 We have to make sure we run yarn generate before publishing it to npm and also that the lib gets included in the npm.

pavanmehta91 commented 4 years ago

Yes I understand. But I believe the generated files, bundles or dist folders should not be included in git. They cause problems and there's no point maintaining there git history. For publishing part I think there are various packages available which can be of help.

tarunmangukiya commented 4 years ago

@aravindballa This looks good to me. Can you please use npm i, so that package-lock.json gets updated?

@ivanahuckova Thanks for testing it out.

tarunmangukiya commented 4 years ago

@pavanmehta91 Thanks for the help. You're right. It should be gitignored.

I am also planning to use GitHub Actions for publishing it.

tarunmangukiya commented 4 years ago

Thanks for the help @aravindballa.

I'm merging the branch and will release new version.