Implement module.parent

[SimenB]

Summary
Fixes #4567. This leaves out id and exports, but it implements require (which I need, and is my motivation for doing this) and filename/id which should be enough for require-dir to work.

module.parent is now fully implemented.

Test plan
New tests added

[cpojer]

Couldn't this look up the parent module in jest-runtime and use its require here, instead of creating a new require implementation?

[SimenB]

If I can access the parent module, then that'd be perfect (as that's what module.parent is in node). I didn't find it though. Halp?

[cpojer]

Aren't we storing this in jest-runtime somewhere?

[SimenB]

we might be, I'll check

[cpojer]

I did not know module.require was a thing. I don't like that it exists :(

[SimenB]

id is the same as filename so added that as well.

Not sure if exports is worth it unless we can just attach the actual parent module to localModule.parent?

[SimenB]

I tried to set localModule.parent now to something from the module cache. Doesn't work properly though.

The first test provides module.parent to modules fails, which I think is correct. Entrypoints don't have a parent so the test is faulty.

The second one makes me kinda sad though. It seems to be one layer to high in the hierarchy. Any ideas?

[cpojer]

Hmm, yeah I don't think I really want to support exposing a runtime for the parent module in Jest. If we wanna add this, could we make it a lazy getter, so that it is only executed when somebody actually accesses module.require?

[SimenB]

Wait, my analysis is off. Since I do module.parent.require the failure actually correct, and my initial implementation was wrong.

[SimenB]

If we wanna add this, could we make it a lazy getter, so that it is only executed when somebody actually accesses module.require?

It's already exposed:

https://github.com/facebook/jest/blob/c44d2c65ea799e4e91b6c22a4beb6028d14149e5/packages/jest-runtime/src/index.js#L523

[cpojer]

Yeah, but you are re-creating the runtime for the parent module in your diff, and I was proposing either not to do that and re-use the existing one, or creating it lazily.

[SimenB]

Do you mean to make module.parent be a lazy getter?

[codecov-io]

Codecov Report

Merging #4614 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #4614   +/-   ##
=======================================
  Coverage   56.17%   56.17%           
=======================================
  Files         194      194           
  Lines        6544     6544           
  Branches        3        3           
=======================================
  Hits         3676     3676           
  Misses       2867     2867           
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-runtime/src/index.js	85.09% <100%> (ø)	⬆️

---

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 c44d2c6...2bd0c6d. Read the comment docs.

[cpojer]

Yes

[SimenB]

I've confirmed using node that the behavior introduced in this PR is correct.

$ cd packages/jest-runtime/src/__tests__/test_root
$ node -p "require('./inner_parent_module').outputString"
This should happen
$ node -p "require('./inner_parent_module').parentFileName"
/Users/simen/Development/jest/packages/jest-runtime/src/__tests__/test_root/inner_parent_module.js

[SimenB]

Actually one thing is wrong, module.parent should be null not undefined for entrypoints.

test-file.js:

console.log(module.parent);

$ node test-file.js
null
$ node -p "require('./test-file')"
Module {
  id: '[eval]',
  exports: {},
  parent: undefined,
  filename: '/Users/simen/Development/jest/[eval]',
  loaded: false,
  children:
   [ Module {
       id: '/Users/simen/Development/jest/test-file.js',
       exports: {},
       parent: [Circular],
       filename: '/Users/simen/Development/jest/test-file.js',
       loaded: false,
       children: [],
       paths: [Array] } ],
  paths:
   [ '/Users/simen/Development/jest/node_modules',
     '/Users/simen/Development/node_modules',
     '/Users/simen/node_modules',
     '/Users/node_modules',
     '/node_modules' ] }
{}

On a second look, if a file is required as an entrypoint, module.parent should be null, but if it's ran directly by node (so [eval]) it's undefined. So I'm not sure what the correct behavior should be here for Jest. Thoughts?

Two issues that I can see that is not related to this PR but to Module in jest in general:

1. module.require is not enumerable. I'll fix that.
2. We're missing loaded in jest's implementation. I'm not sure what it indicates. How do you want it implemented?

EDIT: Opened a separate PR for those 2 last points: #4623

[cpojer]

Nice work @SimenB.

[github-actions]

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.