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 fsevents subdependency for Node 10 compat #1295

Merged
merged 1 commit into from
Jun 30, 2018

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jun 29, 2018

without this update node-gyp failed when running on Node 10.3.0

/cc @trentmwillis

@platinumazure
Copy link
Contributor

Should there also be a change in package.json?

@rwjblue
Copy link
Contributor

rwjblue commented Jun 29, 2018

No, the change is already allowed by the current package.json range. But when you clone the repo and yarn it always gives you the previously locked down version.

@platinumazure
Copy link
Contributor

@rwjblue Understood, but should we set the package.json to a higher version anyway so our dependency list avoids the problematic version even if yarn.lock were removed?

@Turbo87
Copy link
Member Author

Turbo87 commented Jun 29, 2018

@platinumazure fsevents is just a subdependency of some other dependency, so there is no change in the package.json file. If you don't use a lockfile you would have gotten that update automatically, but since the lockfile also locks subdependencies they sometimes need to be updated too.

@Turbo87
Copy link
Member Author

Turbo87 commented Jun 29, 2018

whoops, missed that @rwjblue had already replied.

fsevents is brought in by chokidar, and yes an update of that dependency would be good, but since their current version is 2.0.4 which has a major bump compared to the 1.7.0 we use, this is a slightly larger undertaking to upgrade.

@platinumazure
Copy link
Contributor

Thanks, that answers my questions.

Copy link
Member

@trentmwillis trentmwillis left a comment

Choose a reason for hiding this comment

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

Thanks!

@trentmwillis trentmwillis merged commit 64e8f24 into qunitjs:master Jun 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants