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

fix: revert usage of Escalade and yargs@16 as they breaks Node 13 #10599

Merged
merged 8 commits into from
Oct 6, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Oct 6, 2020

Summary

Fixes #10596

This reverts commit 0a9e77d.

Let's land it for Jest 27 instead

Test plan

🤷

@TrySound
Copy link
Contributor

TrySound commented Oct 6, 2020

It's not only escalade. Reported comment was about yargs.

@TrySound
Copy link
Contributor

TrySound commented Oct 6, 2020

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

Good call, I'll rollback yargs as well

@SimenB SimenB changed the title fix: revert usage of Escalade as it breaks Node 13 fix: revert usage of Escalade and yargs@16 as it breaks Node 13 Oct 6, 2020
@TrySound
Copy link
Contributor

TrySound commented Oct 6, 2020

node-notifier also depends on uuid 8.3 with es modules support

@TrySound
Copy link
Contributor

TrySound commented Oct 6, 2020

get-package-type from @istanbuljs/load-nyc-config has exports field

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

Bah. I'll install node 13 and verify

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

@istanbuljs/load-nyc-config doesn't matter as it's never invoked (we pass config explicitly).

Rolling node-notifier back to 7.0.1 works (with jest --notify --coverage) on v13.2.0, but reopens #9701. @mikaelbr thoughts on using uuid@7 in latest node-notifier? I know supporting EOL versions of node sucks, so I understand if you don't want to 🙂

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

Almost worth a quick Jest 27 after this just reverting this commit and updating engines to exclude v13... Almost.

@SimenB SimenB changed the title fix: revert usage of Escalade and yargs@16 as it breaks Node 13 fix: revert usage of Escalade, yargs@16 and node-notifier@8 as it breaks Node 13 Oct 6, 2020
@mikaelbr
Copy link
Contributor

mikaelbr commented Oct 6, 2020

@SimenB Using uuid would be fine by me, can also add node 13 to test engines. Not able to do it right away, though, as I am on a short vacation without access to a laptop.

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

It's specifically old versions of node 13, not the latest ones. Probably not worth it

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

That said, we've been installing a version of node-notifier with uuid@8 for months. It's lazy loaded, so it only impacts people who do --notify. We could just keep it at that version since nobody has complained about it? The other 2 deps are always used though, so we should revert those to working versions.

Jest itself will use exports in the next major, so all of this is void at that point anyways.

@SimenB
Copy link
Member Author

SimenB commented Oct 6, 2020

I'll keep node-notifier at 8 for now. I think asking people to use a non-EOL release of node if they want notifications working is fine.

@SimenB SimenB changed the title fix: revert usage of Escalade, yargs@16 and node-notifier@8 as it breaks Node 13 fix: revert usage of Escalade and yargs@16 as they breaks Node 13 Oct 6, 2020
@SimenB SimenB merged commit 7e51ac1 into jestjs:master Oct 6, 2020
@SimenB SimenB deleted the revert-escalade branch October 6, 2020 10:44
@lukeed
Copy link
Contributor

lukeed commented Oct 16, 2020

This has been resolved in escalade and yargs

@SimenB
Copy link
Member Author

SimenB commented Oct 16, 2020

Ah, great news! I still think we'll wait until Jest 27 regardless (#9921)

@lukeed
Copy link
Contributor

lukeed commented Oct 16, 2020

No problem, just letting you know :) Everyone has/had this problem thanks to Node 13.x experiments.

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

Update engine requirement
5 participants