Jest: expect NPM package unusable in browser

[jods4]

🐛 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]

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]

Ah, it's https://github.com/rollup/rollup/issues/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]

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]

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]

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]

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]

Ok, i will fix tests and send PR :)

[leonardovillela]

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

[SimenB]

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]

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

[luisSpinola]

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

[luisSpinola]

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]

Fixed in #7548 (not released)

[SimenB]

Reverted: #7602