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

feat: override module.createRequire #9469

Merged
merged 13 commits into from
Feb 1, 2020
Merged

feat: override module.createRequire #9469

merged 13 commits into from
Feb 1, 2020

Conversation

doniyor2109
Copy link
Contributor

PR for #9426

@codecov-io
Copy link

codecov-io commented Jan 26, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@af97309). Click here to learn what that means.
The diff coverage is 86.36%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #9469   +/-   ##
========================================
  Coverage          ?     65%           
========================================
  Files             ?     283           
  Lines             ?   12131           
  Branches          ?    3002           
========================================
  Hits              ?    7886           
  Misses            ?    3604           
  Partials          ?     641
Impacted Files Coverage Δ
packages/pretty-format/src/index.ts 95.03% <ø> (ø)
packages/jest-util/src/createDirectory.ts 0% <0%> (ø)
packages/jest-snapshot/src/utils.ts 95.6% <100%> (ø)
packages/jest-runtime/src/index.ts 65.22% <100%> (ø)
packages/jest-reporters/src/coverage_reporter.ts 51.21% <100%> (ø)
packages/jest-transform/src/ScriptTransformer.ts 69.64% <84.61%> (ø)

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 af97309...ecce093. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

nice, thank you! I think we should add some more validation about the shape of the passed in path, other than that this lgtm

packages/jest-runtime/src/index.ts Outdated Show resolved Hide resolved
packages/jest-runtime/src/index.ts Show resolved Hide resolved
packages/jest-runtime/src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

this is great, thank you!

packages/jest-runtime/src/index.ts Outdated Show resolved Hide resolved
yarn.lock Outdated
resolved "https://registry.yarnpkg.com/@types/node/-/node-13.7.0.tgz#b417deda18cf8400f278733499ad5547ed1abec4"
integrity sha512-GnZbirvmqZUzMgkFn70c74OQpTTUcCzlhQliTzYjQMqg+hVKcDnxdL19Ne3UdYzdMA/+W3eb646FWn/ZaT1NfQ==

"@types/node@>= 8":
Copy link
Member

Choose a reason for hiding this comment

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

roll back change here?

Copy link
Contributor Author

@doniyor2109 doniyor2109 Feb 1, 2020

Choose a reason for hiding this comment

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

I have upgraded @types/node to 13.7 because this version includes syncBuiltinESMExports in module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have reverted changes back. I just used another type

Copy link
Member

Choose a reason for hiding this comment

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

oh! I just didn't want 2 versions of it, could have deduped it to the newer ones. Lemme play with it locally. Thanks!

@SimenB SimenB mentioned this pull request Feb 1, 2020
21 tasks
@SimenB SimenB changed the title Origin/override module create require feat: override module.createRequire Feb 1, 2020
@SimenB SimenB merged commit 523ba3b into jestjs:master Feb 1, 2020
@SimenB
Copy link
Member

SimenB commented Feb 1, 2020

Thank you so much @doniyor2109!

haoqunjiang added a commit to vuejs/vue-cli that referenced this pull request Jan 20, 2021
1. createRequire should have been properly handled in newer Jest
versions: jestjs/jest#9469
2. We don't test migrators with Jest mock modules anymore

So it's safe to skip that condition
@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.

4 participants