jest-worker: should not expose `.default` babel interop

Created on 15 Mar 2018  ·  21Comments  ·  Source: facebook/jest

import a from 'jest-worker' of course, works fine, because it's using babel on both ends.

However, const b = require('jest-worker') results in a !== b, and to get at the proper value, you need to do b.default.

Entry points should not expose babel interop details like .default - can https://github.com/facebook/jest/blob/master/packages/jest-worker/src/index.js use module.exports = instead of export default (or can you use the add-module-exports transform?

Enhancement Help Wanted good first issue

Most helpful comment

We're exporting multiple things (which I don't wanna change), so the babel plugin won't work. A named export is probably our only option, although it's a breaking change. Instead of export default class JestWorker we can do export class JestWorker. Possibly name it Worker instead as we don't need to repeat the package name in the export

All 21 comments

add-module-exports sounds good to me, I agree it's annoying having to do .default in node (and probably not good for real ESM support in node).

PR welcome! :grinning:

@SimenB Here is a proposal: https://github.com/facebook/jest/pull/5811

See comment in PR

After a year with an open PR, i think this is a really user-hostile decision.

The PR didn't work properly. Using the plugin was discussed a bit in #7554 and it seems like people have been burnt by add-module-exports before. And e.g. Babel itself removed it for their 7 release.

There’s lots of alternatives including a manual entry point that does the un-.defaulting.

As for Babel themselves, it’s reasonable for Babel to expect consumers to couple to Babel interop, it’s not reasonable for “not Babel” to do so.

Yeah, that's an option. Do you know if that would still work for people doing import? I guess we could do module.exports = require('./build/index'); (unpacking the default in some way) and as long as types in package.json still pointed to build/index.d.ts it would work?

Yes, it would work for both.

enzyme does this with all it top-level entry points, for example.

That sounds perfect then. Reopening 😀

The link at the top is broken. Should be updated to: https://github.com/facebook/jest/blob/master/packages/jest-worker/src/index.ts

@SimenB @jeysal I would love to take this up as my first issue if possible

@ljharb can i use this https://www.npmjs.com/package/babel-plugin-add-module-exports or do i need to explicitly, change to export default to module.exports

@saenglert as long as the main entry point of the package only has one default export, that transform should do the job.

@ljharb please i dont really understand your explanation do you mean this transformation export default to module.exports and not using the babel package

We're exporting multiple things (which I don't wanna change), so the babel plugin won't work. A named export is probably our only option, although it's a breaking change. Instead of export default class JestWorker we can do export class JestWorker. Possibly name it Worker instead as we don't need to repeat the package name in the export

@SimenB i'm pretty sure you could still change things by attaching all the named exports to the default export, and explicitly add __esModule: true, and it wouldn't be a breaking change - it'd be pretty gross tho, to be sure.

Just ditch the default export, I don't think it's worth it to go through weird hoops to support both and keep TypeScript happy.

@SimenB I changed it to the named export, got all tests passing at the jest-worker package and build successful but i cant access jest-worker outside the package using a named import
Link to my branch: https://github.com/MLH-Fellowship/jest/tree/fix-export-default-worker
test-worker

@anje123 if you open up a PR with your WIP stuff I can take a look 🙂

@SimenB created a PR #10623 Thanks

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ticky picture ticky  ·  3Comments

kgowru picture kgowru  ·  3Comments

stephenlautier picture stephenlautier  ·  3Comments

samzhang111 picture samzhang111  ·  3Comments

ianp picture ianp  ·  3Comments