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

adds missing node builtin modules to jest-resolve package #4740

Merged
merged 1 commit into from
Oct 22, 2017

Conversation

CNDW
Copy link
Contributor

@CNDW CNDW commented Oct 21, 2017

This removes the usage of the is-builtin-module dependency in the jest-resolve package to utilize a less strict method of checking for builtin node dependencies.

The change is to fix #4677 which causes jest to break when any node package is making use of a builtin filtered out by the is-builtin-module package. The author has made clear that they will not include the packages so jest should not be utilizing this package any longer.

Using the following repo to test for the issue

Before

travis@travis-XPS-15-9560:~/Projects/bugreport$ yarn test
yarn test v1.0.1
$ jest
 FAIL  ./test.spec.js
  Some thing :
    ✕ should be true (26ms)

  ● Some thing : › should be true

    Cannot find module '_http_common' from 'stream.js'

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:191:17)
      at Object.<anonymous> (node_modules/spdy/lib/spdy/stream.js:13:20)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.235s
Ran all test suites.
error Command failed with exit code 1.

After the fix is in place

travis@travis-XPS-15-9560:~/Projects/bugreport$ yarn test
yarn test v1.0.1
$ jest
 PASS  ./test.spec.js
  Some thing :
    ✓ should be true (65ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.301s, estimated 1s
Ran all test suites.
Done in 0.56s.

@CNDW CNDW changed the title changes method of determining builtin modules to include missing buil… adds missing node builtin modules to jest-resolve package Oct 21, 2017
@cpojer cpojer merged commit 7db15f0 into jestjs:master Oct 22, 2017
@cpojer
Copy link
Member

cpojer commented Oct 22, 2017

Nice work! Thanks for sending a PR and for adding tests.

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

5 builtins missing from jest-resolve
3 participants