Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export CommonJS module #31

Closed
wata727 opened this issue Jun 28, 2019 · 6 comments
Closed

Export CommonJS module #31

wata727 opened this issue Jun 28, 2019 · 6 comments

Comments

@wata727
Copy link

wata727 commented Jun 28, 2019

Congratulations on the v4 release 🎉

Btw, our Jest tests are broken with the classcat v4 😢

/home/wata727/ghq/github.com/wata727/sideci/node_modules/classcat/src/index.js:3                         
    export default function cc(names) {                                                                      
    ^^^^^^                                                                                                   

    SyntaxError: Unexpected token export

      13 |   readonly className?: string
      14 |   readonly onClick?: (children: string) => void
    > 15 |   readonly children: string
         |                                    ^
      16 |   readonly dismissible?: boolean
      17 |   readonly button?: boolean
      18 |   readonly "data-spec"?: string

      at ScriptTransformer._transformAndBuildScript (../node_modules/@jest/transform/build/ScriptTransformer.js:471:17)
      at ScriptTransformer.transform (../node_modules/@jest/transform/build/ScriptTransformer.js:513:25)
      at Object.<anonymous> (src/cheshire/components/Chip/index.tsx:15:36)

I guess that's probably because the main target is now an ES module. But Jest doesn't support ES modules natively. See jestjs/jest#4842

My team thinks it is not a good idea to introduce Babel just because of this problem... Could you also support CommonJS style modules? Thank you.

@jorgebucaran
Copy link
Owner

@wata727 Do you mean changing main to a UMD or would it be enough with creating a new target like dist/classcat.umd.js?

@wata727
Copy link
Author

wata727 commented Jun 28, 2019

I think that the least impact from the current release is to add a new target. We plan to use moduleNameMapper to replace it during testing. See https://jestjs.io/docs/en/configuration.html#modulenamemapper-object-string-string

{
  "moduleNameMapper": {
    "classcat": "classcat/dist/classcat.umd.js"
  }
}

@jorgebucaran
Copy link
Owner

@wata727 That's a good idea. Would you like to send me a PR? Otherwise, I can get to it later today.

@wata727
Copy link
Author

wata727 commented Jun 28, 2019

Sure! I'll try it.

@jorgebucaran
Copy link
Owner

@wata727 Fixed with 378cabe, but do check, please. 🎉

@wata727
Copy link
Author

wata727 commented Jun 29, 2019

@jorgebucaran It worked well. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants