Skip to content

Conversation

@jablko
Copy link
Contributor

@jablko jablko commented Aug 26, 2021

For ESM tests, jestAdapter() calls Runtime.unstable_importModule() -> Runtime.loadEsmModule():
https://github.com/facebook/jest/blob/d38156ccd7a68e52372b4eb6fc02afbadd5b703e/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts#L74-L78

It doesn't call Runtime.requireModule() -> Runtime._loadModule() -> Runtime._execModule(), so it doesn't set Runtime._mainModule/require.main:
https://github.com/facebook/jest/blob/d38156ccd7a68e52372b4eb6fc02afbadd5b703e/packages/jest-runtime/src/index.ts#L1205-L1207

Consequently require.main is null:

 FAIL  test/index.test.js
  ● Test suite failed to run

    TypeError: Cannot read property 'filename' of null

I haven't investigated whether this changed in #10621 (whether previously require.main was always undefined, or only when using isolateModules(), ∴ regression?). Either way, this PR proposes to copy this._mainModule = module to the ESM code path.

I'm not sure where is the "right" location -- between loadEsmModule() and linkAndEvaluateModule() seemed analogous to the existing location in Runtime._execModule()?

@codecov-commenter
Copy link

Codecov Report

Merging #11790 (3847dc7) into master (d38156c) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11790      +/-   ##
==========================================
- Coverage   69.01%   69.00%   -0.01%     
==========================================
  Files         312      312              
  Lines       16346    16348       +2     
  Branches     4742     4743       +1     
==========================================
  Hits        11281    11281              
- Misses       5037     5039       +2     
  Partials       28       28              
Impacted Files Coverage Δ
packages/jest-runtime/src/index.ts 58.62% <0.00%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d38156c...3847dc7. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Aug 27, 2021

Where you added it here seems fine to me 🙂 Could you add a test as well? Plus a changelog entry

@jablko jablko mentioned this pull request Aug 28, 2021
@SimenB
Copy link
Member

SimenB commented Sep 29, 2021

@jablko ping 🙂

@SimenB
Copy link
Member

SimenB commented Feb 24, 2022

Based on https://nodejs.org/api/modules.html#accessing-the-main-module

When the entry point is not a CommonJS module, require.main is undefined, and the main module is out of reach.

I think the current behaviour is correct when the entry point is ESM

@SimenB SimenB closed this Feb 24, 2022
@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 Mar 27, 2022
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.

4 participants