-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
refactor(resolve): replace read-pkg-up with escalade #10558
Conversation
Ref https://packagephobia.com/result?p=read-pkg-up https://packagephobia.com/result?p=escalade Read-pkg-up package is quite big and have a lot of dependencies. It also handles cases not necessary for jest. Beautiful JSON parsing errors are not relevant and can be replaced with builtin JSON.parse.
Aha, looks like testing on 13.2 is not a good idea. Native ESM support is broken until 13.7. The issue is the same as in many packages with es modules support.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Left some inline comments. In addition to those, could you add a changelog entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
for anyone stumbling across this issue due to this error:
you need to update the version of node that you're using to >=13.7 |
Let's revert and re-land for Jest 27. At that point we'll drop support for Node 13 anyways (13 has been EOL since June). |
@SimenB Are you gonna revert and escalade and yargs as well? Are you sure any more dependencies won't add it? |
|
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. |
Summary
Ref https://packagephobia.com/result?p=read-pkg-up https://packagephobia.com/result?p=escalade
Read-pkg-up package is quite big and have a lot of dependencies.
It also handles cases not necessary for jest. Beautiful JSON parsing
errors are not relevant and can be replaced with builtin JSON.parse.
Test plan
Not sure about this. Looks like es modules support was not covered by tests.