Also ship with commonJS

brightloudnoise commented 5 years ago

It would be great to also ship this module with CommonJS. I ran into an issue using it with next.js

https://github.com/zeit/next.js/issues/7913#issuecomment-511397930

tarunmangukiya commented 5 years ago

Thanks for suggestion! Can you create a PR for it?

BjoernRave commented 4 years ago

I have the same problem, would love this to get solved :)

slugger7 commented 4 years ago

Failing my jest tests on react 😢

ivanahuckova commented 4 years ago

We have done workaround in the repo where we use Unicons. This fixes failing Jest tests -https://github.com/grafana/grafana/blob/master/jest.config.js#L1-L7. And because Unicons (and some of the other packages that we use) use es2015 syntax, we needed to transpile them to get rid of unsupported syntax - https://github.com/grafana/grafana/blob/master/scripts/webpack/webpack.common.js#L4-L70. Hope it helps! 🙂

tarunmangukiya commented 4 years ago

Thank you @ivanahuckova for the help. Do you know how can we fix this issue in Unicons itself?

ivanahuckova commented 4 years ago

@tarunmangukiya I was actually writing you an email regarding this. 🙂If you would like, I can send a PR for this. Do you have any preference for the solution? I was thinking about using babel or rollup.

tarunmangukiya commented 4 years ago

@ivanahuckova Glad to hear that. Yes, for sure. I'll be more than happy to have a PR. I am good with either babel or rollup. I have heard a lot about rollup recently.

ivanahuckova commented 4 years ago

Awesome! I will try to look at it this evening and I'll keep you updated. 🙂

tarunmangukiya commented 4 years ago

@ivanahuckova @slugger7 Can you test this one?

https://github.com/Iconscout/react-unicons/pull/9

tarunmangukiya commented 4 years ago

Thank you guys for your patience. We've released new update with CommomJS Support. 🎉

Special thanks to @aravindballa for efforts. Thank you @ivanahuckova @pavanmehta91 for testing it.

Do try it out: https://github.com/Iconscout/react-unicons/releases/tag/v1.1.0

sumit-enspara commented 4 years ago

Hi team, I tried to run jest test cases using version 1.0.1 of react-unicons in a project bootstraped using Create react app. Since commonjs was not supported I got the below error. To resolve this error, I added --transformIgnorePatterns \"node_modules/(?!@iconscout/react-unicons)/\" to test script and it worked fine.

({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import React from 'react';
                                                                                                    ^^^^^
    SyntaxError: Unexpected identifier
      3 | import { MdDomain, MdTerrain, MdLayers, MdWeekend, MdWarning } from 'react-icons/md'
    > 4 | import UilAngleDown from '@iconscout/react-unicons/icons/uil-angle-down'
        | ^
      5 | import UilAngleRight from '@iconscout/react-unicons/icons/uil-angle-right'
      at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:537:17)
      at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:579:25)
      at Object.<anonymous> (src/components/NavigationTreeBranch/NavigationTreeBranch.jsx:4:1)

While I upgraded the @iconscout/react-unicons version to 1.1.0 after the recent release, I still face the same error. Am I missing some configurations?

slugger7 commented 4 years ago

After a few tests on my side I am also still facing this problem 😢 Could not test sooner either.

tarunmangukiya commented 4 years ago

@sumit-enspara @slugger7 Have you checked config by @ivanahuckova? They are also using jest for testing.

olivierpichon commented 4 years ago

Hi guys, @ivanahuckova 's config is a workaround to get to transpile the module. It is unfortunately not a real solution. It would be amazing if the package would work out of the box when using something as common as create-react-app, without having to force the module transpilation.

ivanahuckova commented 4 years ago

Hi @tarunmangukiya, it looks like the published package doesn't contain lib/cjs/index.js file. https://github.com/Iconscout/react-unicons/pull/9

tarunmangukiya commented 4 years ago

@ivanahuckova Yeah, I forgot to manage npmignore. I am releasing the new version today with new icons. Will fix it and release this package too.

tarunmangukiya commented 4 years ago

Hey guys @slugger7 @olivierpichon, I've released new v1.1.1 just now. Can you try with this version?

sumit-enspara commented 4 years ago

@tarunmangukiya While I installed the latest version tagged 1.1.1, do I still need to add transformIgnorePatterns to jest configs?

Without those jest configs I still get the same below error: https://github.com/Iconscout/react-unicons/issues/2#issuecomment-620552606

tarunmangukiya commented 4 years ago

I think you won't need it @sumit-enspara.

Iconscout / react-unicons

Also ship with commonJS #2