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

Add module.loaded, and module.require should not be enumerable #4623

Merged
merged 3 commits into from
Oct 7, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Oct 7, 2017

Summary
As mentioned in #4614 (comment) there are 2 ways that module in normal node and module in jest diverge (at the top level, #4614 addresses module.parent being faked).

  1. module.requireshould not be enumerable.
  2. module.loaded is missing in jest's implementation. (https://nodejs.org/api/modules.html#modules_module_loaded)

This PR fixes both of those issues.

Test plan
New test added

@SimenB SimenB force-pushed the enumerable-module-require branch from 0c98697 to 4e0bb4c Compare October 7, 2017 10:20
@SimenB SimenB mentioned this pull request Oct 7, 2017
@SimenB SimenB force-pushed the enumerable-module-require branch 3 times, most recently from d7848d1 to baadeae Compare October 7, 2017 11:31
@@ -29,13 +29,14 @@ import {run as cilRun} from './cli';
import {options as cliOptions} from './cli/args';

type Module = {|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be type Module = typeof module to get the type definitions from flow itself (it would have yelled at us for missing loaded, for instance). A bit more work though as it also doesn't like that require is added the way it is in this PR. Can revisit later

@codecov-io
Copy link

codecov-io commented Oct 7, 2017

Codecov Report

Merging #4623 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #4623      +/-   ##
=========================================
+ Coverage   56.18%   56.2%   +0.01%     
=========================================
  Files         194     194              
  Lines        6546    6548       +2     
  Branches        3       3              
=========================================
+ Hits         3678    3680       +2     
  Misses       2867    2867              
  Partials        1       1
Impacted Files Coverage Δ
packages/jest-runtime/src/index.js 85.21% <100%> (+0.11%) ⬆️

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 7b0e108...32e302c. Read the comment docs.

@cpojer
Copy link
Member

cpojer commented Oct 7, 2017

please rebase :)

@SimenB SimenB force-pushed the enumerable-module-require branch from baadeae to ea071a3 Compare October 7, 2017 12:09
@SimenB SimenB force-pushed the enumerable-module-require branch from ea071a3 to 32e302c Compare October 7, 2017 12:10
@SimenB
Copy link
Member Author

SimenB commented Oct 7, 2017

Rebased!

@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 13, 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.

4 participants