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

Fix: jest-worker exposing .default babel interop #5811

Closed

Conversation

felippenardi
Copy link

Summary

The issue #5803 raised up by @ljharb:

import a from 'jest-worker' 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.

Test plan

I will appreciate some help to evaluate if this is a valid change and if it solves the problem.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@felippenardi
Copy link
Author

Most of the tests are failing because of TypeError: runtime.requireInternalModule(...).default is not a function.

That is why I removed the .default from local module imports, but it didn't help.

Can you guys give any guidance needs to be done here?

@ljharb
Copy link
Contributor

ljharb commented Mar 16, 2018

Note that add-module-exports only works on files that have just a default export; files that also have named exports need to either retain the .default, or avoid export altogether and attach the named exports as properties on the module.exports default export.

@felippenardi
Copy link
Author

@ljharb So I can undo the last commit and remove .default only from the modules that are just exporting default export. Is that correct?

@ljharb
Copy link
Contributor

ljharb commented Mar 16, 2018

That’s the shorter-term way to get unblocked :-)

@rickhanlonii
Copy link
Member

@felippenardi want to wrap this up so we can get into Jest 23?

@SimenB
Copy link
Member

SimenB commented Mar 19, 2019

We're not gonna provide an interop here. I know it's ugly in CJS, but I don't think going down this track is worth it in the long run. We wanna move to ESM for all modules in this repo

@SimenB SimenB closed this Mar 19, 2019
@SimenB
Copy link
Member

SimenB commented Mar 19, 2019

As talked about in the issue, a different approach using an intermediary file to unwrap the default should be a feasible solution

@github-actions
Copy link

This pull request 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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants