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

chore: update fsevents #11028

Merged
merged 2 commits into from Jan 26, 2021
Merged

chore: update fsevents #11028

merged 2 commits into from Jan 26, 2021

Conversation

cmdcarini
Copy link
Contributor

@cmdcarini cmdcarini commented Jan 25, 2021

Summary

Attempts to address Jest hanging on test execution when running on macOS by updating the nested optional fsevents dependency in the jest-haste-map module to include upstream changes that resolved the underlying issue.

Test plan

When running latest stable version of Jest (v26.6.3), manually overriding downstream dependency fsevents to v2.2.1 by doing the following, resolves hang up issues, and maintains Jest functionality.

npm i -D fsevents@2.2.1
npm dedupe
npm uninstall -D fsevents

@facebook-github-bot
Copy link
Contributor

Hi @cmdcarini!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Contributor

@merceyz merceyz left a comment

Choose a reason for hiding this comment

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

Semver already takes care of this bump for users so this shouldn't be needed unless it's an issue in this repo as well, left some comments inline either way

packages/jest-haste-map/package.json Outdated Show resolved Hide resolved
@@ -34,7 +34,7 @@
"slash": "^3.0.0"
},
"optionalDependencies": {
"fsevents": "^2.1.2"
"fsevents": "^2.2.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to run yarn to update the lockfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should (hopefully) be done!

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like you used Yarn 1 to do that instead of the checked in version, how old is your Yarn 1 version since it didn't pick up on the yarn-path setting?

Copy link
Member

@SimenB SimenB Jan 26, 2021

Choose a reason for hiding this comment

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

@merceyz I tried to fix it, but the patch in yarn v2 seems broken.

➤ YN0066: │ fsevents@patch:fsevents@npm%3A2.3.1#builtin<compat/fsevents>::version=2.3.1&hash=127e8e: Cannot apply hunk #1
➤ YN0000: │     }
➤ YN0000: │
➤ YN0028: │ -   let instance = Native.start(path, handler);
➤ YN0028: │ +   let VFS = require('./vfs');
➤ YN0028: │ +   let vfs = new VFS(path);
➤ YN0028: │ +   let instance = Native.start(vfs.resolvedPath, vfs.wrap(handler));
➤ YN0000: │     if (!instance) throw new Error(`could not watch: ${path}`);
➤ YN0000: │     return () => {

(same for 2.2.1, fwiw)

Copy link
Contributor

@merceyz merceyz Jan 26, 2021

Choose a reason for hiding this comment

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

That was fixed in yarnpkg/berry#2078 (yarn set version latest)

Copy link
Member

@SimenB SimenB Jan 26, 2021

Choose a reason for hiding this comment

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

Oh, the fork was super outdated (over 120 commits behind) and had an old version of yarn. Merging in master fixed the issue 👍

@SimenB SimenB changed the title build: update fsevents to include stability fixes chore: update fsevents Jan 26, 2021
@SimenB SimenB merged commit baf9f99 into jestjs:master Jan 26, 2021
@SimenB
Copy link
Member

SimenB commented Jan 26, 2021

Thanks @cmdcarini!

slorber pushed a commit to jest-website-migration/jest that referenced this pull request Feb 11, 2021
@arnulfojr
Copy link

Hi, do you have a release date for this?
Can we have maybe 26.6.4, instead of waiting for v27?
The jest tests keep hanging with the current version of fsevents

@SimenB
Copy link
Member

SimenB commented Feb 18, 2021

You don't need it, latest version is in semver range. just delete the entry from you lockfile

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

None yet

5 participants