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

expect NPM package unusable in browser #7109

Closed
jods4 opened this issue Oct 5, 2018 · 15 comments · Fixed by #7661
Closed

expect NPM package unusable in browser #7109

jods4 opened this issue Oct 5, 2018 · 15 comments · Fixed by #7661

Comments

@jods4
Copy link

jods4 commented Oct 5, 2018

🐛 Bug Report

The expect NPM package is not usable in browser, although it has a browser: build-es5/index.js field.

To Reproduce

Add this to a static page:

<script src="/node_modules/expect/build-es5/index.js"></script>

It almost works, with the sole exception that the very last line fails:
module.exports = expect
Because there's no global module object.

Expected behavior

Should not throw and expose a global expect object.

If you detect that there's no module but a window instead, doing window.expect = expect makes everything work perfectly.

@SimenB
Copy link
Member

SimenB commented Oct 6, 2018

That's odd, we are outputting umd modules for it: https://github.com/facebook/jest/blob/d59a4d33b6aff50df3f2a307d71797a5d0e5f298/scripts/browserBuild.js#L70

Do we do anything else that messes up the transformation?

BTW, it works if you bundle up your tests using e.g. browserify, but we should definitely work as a script tag

@SimenB
Copy link
Member

SimenB commented Oct 6, 2018

Ah, it's rollup/rollup#1646 combined with https://github.com/facebook/jest/blob/d59a4d33b6aff50df3f2a307d71797a5d0e5f298/packages/expect/src/index.js#L389

Since our next release is a major, we should probably go through and remove all module.exports usage in favor of export default. It will force people using require to do .default, but hopefully it's not too big of a deal

@jods4
Copy link
Author

jods4 commented Oct 6, 2018

I have hacked a global module object to make it work and it seems that the only problem.

Glad to hear you'll fix it, thanks!

@leonardovillela
Copy link
Contributor

Hello Guys, can i take this issue? Can you provide some guidance about implementation? Change module.exports to export default in only in package expect?

@leonardovillela
Copy link
Contributor

Can i take this issue? Can you provide some guidance? Changes to module.exports to export default should be made only in expect package?

@SimenB
Copy link
Member

SimenB commented Oct 22, 2018

I've got a branch with some of the work done, you can work off of that: https://github.com/SimenB/jest/tree/esm-export

I think it's just a matter of fixing the tests, and you should be good to go 🙂

@leonardovillela
Copy link
Contributor

Ok, i will fix tests and send PR :)

@leonardovillela
Copy link
Contributor

I should add dynamic imports in interopRequireDefault that contain // $FlowFixMe: dynamic import in this PR?

@SimenB
Copy link
Member

SimenB commented Oct 23, 2018

Do you mean support import('my-thing')? If so, no. // $FlowFixMe is us telling flow to ignore the fact that it's impossible to statically verify the type of the module required. It's not a TODO type comment 🙂

@leonardovillela
Copy link
Contributor

Okay, so I'm not going to remove it.

@luisSpinola
Copy link

@jods4 Im having trouble reproducing this bug, could someone help me in a beginner level, thanks in advance.

@luisSpinola
Copy link

I managed to reproduce it, the 1st slash "/" in the src of the script was the problem for me if anyone is having trouble too.

@SimenB
Copy link
Member

SimenB commented Jan 2, 2019

Fixed in #7548 (not released)

@SimenB SimenB closed this as completed Jan 2, 2019
@SimenB
Copy link
Member

SimenB commented Jan 11, 2019

Reverted: #7602

@SimenB SimenB reopened this Jan 11, 2019
@SimenB SimenB removed this from the Jest 24 milestone Jan 11, 2019
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants