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

Update package.json to install fsevents on node >= 12 #38615

Merged
merged 1 commit into from Mar 21, 2020
Merged

Update package.json to install fsevents on node >= 12 #38615

merged 1 commit into from Mar 21, 2020

Conversation

yashLadha
Copy link
Contributor

@yashLadha yashLadha commented Mar 2, 2020

Summary

When executing the command yarn install in the project root directory using node 12. It breaks because fsevents exits with code 1. There is already an issue on node repo which resolves this stated in this comment

Other Information

Closes #38612

@yashLadha yashLadha changed the title Update packag.json to install fsevents on node >= 12 Update package.json to install fsevents on node >= 12 Mar 2, 2020
@rails-bot rails-bot bot added the railties label Mar 2, 2020
@yashLadha
Copy link
Contributor Author

Need to pass --no-optional in docker file so that optional dependencies doesn't get installed

@eugeneius
Copy link
Member

It looks like adding a resolution prevents optional dependencies from being ignored when they can't be installed on the current system:

https://buildkite.com/rails/rails/builds/67339#654dc08d-e7ab-44bb-9936-ad6a725a1ce7/1512-1516

error fsevents@1.2.11: The platform "linux" is incompatible with this module.
error Found incompatible module.

Here's what happens on master, with no resolution:

https://buildkite.com/rails/rails/builds/67326#872684c5-29d4-46f9-8ade-99e4098e8c09/224-236

info fsevents@1.2.11: The platform "linux" is incompatible with this module.
info "fsevents@1.2.11" is an optional dependency and failed compatibility check. Excluding it from installation.

That difference in behaviour is a bug in Yarn, tracked here: yarnpkg/yarn#7590

I think we can work around it by adding the resolution, running yarn install (on a Mac), committing the updated yarn.lock and throwing away the changes to package.json.

@eugeneius eugeneius removed the railties label Mar 3, 2020
@eugeneius
Copy link
Member

I don't think we should fix the problem by passing --no-optional when running Yarn on CI, since it would force contributors installing dependencies locally on Linux or Windows to also use the flag.

@yashLadha
Copy link
Contributor Author

Did as suggested @eugeneius

Copy link
Member

@eugeneius eugeneius left a comment

Choose a reason for hiding this comment

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

Thanks @yashLadha! This looks good to me.

I can confirm that on master running yarn install on macOS with Node 12.16.0 fails to compile the fsevents package as described in #38612, but with this change the package is compiled successfully.

@yashLadha
Copy link
Contributor Author

@eugeneius Can this be merged?

@eugeneius eugeneius merged commit 432fe86 into rails:master Mar 21, 2020
@yashLadha yashLadha deleted the chore/node12_fsevents branch August 19, 2020 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn install breaks on node12 LTS
2 participants