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

Yarn engine requirement #740

Merged
merged 2 commits into from Sep 1, 2017
Merged

Yarn engine requirement #740

merged 2 commits into from Sep 1, 2017

Conversation

@javan
Copy link
Member

@javan javan commented Sep 1, 2017

Adds yarn requirement to package.jsons engines:

webpacker/package.json

Lines 9 to 11 in 09247aa

"engines": {
"node": ">=6.0.0",
"yarn": ">=0.25.2"

Updates webpacker:check_yarn and webpacker:check_node to read requirements from package.json so they're not duplicated (and easily forgotten):

pkg_path = Pathname.new("#{__dir__}/../../../package.json").realpath
yarn_requirement = JSON.parse(pkg_path.read)["engines"]["yarn"]
requirement = Gem::Requirement.new(yarn_requirement)
version = Gem::Version.new(yarn_version)
unless requirement.satisfied_by?(version)

@javan javan requested review from gauravtiwari and dhh Sep 1, 2017
@javan javan mentioned this pull request Sep 1, 2017
node_requirement = JSON.parse(pkg_path.read)["engines"]["node"]

requirement = Gem::Requirement.new(node_requirement)
version = Gem::Version.new(node_version.strip.tr("v", ""))

This comment has been minimized.

@gauravtiwari

gauravtiwari Sep 1, 2017
Member

Shall we extract this part inside version module? And then perhaps use that to check inside tasks?

if Webpacker::Version.required_node_version?(version)
if Webpacker::Version.required_yarn_version?(version)

This comment has been minimized.

@javan

javan Sep 1, 2017
Author Member

I'd rather not add a new API that's only used by rake tasks, but 👍 if there's ever a broader need.

This comment has been minimized.

@gauravtiwari

gauravtiwari Sep 1, 2017
Member

Cool, makes sense 👍

@javan javan merged commit 9d29ddd into master Sep 1, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@javan javan deleted the yarn-engine-requirement branch Sep 1, 2017
@phuwanart

This comment has been minimized.

Copy link

@phuwanart phuwanart commented on package.json in d5d0bdd Sep 6, 2017

Heroku use yarn 0.22.0, Is it necessary to add a yarn version? Because of this, it causes problems when deploy to Heroku.

This comment has been minimized.

Copy link
Member Author

@javan javan replied Sep 6, 2017

Is it necessary to add a yarn version?

See #727

Because of this, it causes problems when deploy to Heroku

See #739

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.