Detect Yarn the same way the installer does - #673
Merged
Conversation
tricknotes
force-pushed
the
unify-yarn-detection
branch
from
September 7, 2026 12:22
8ff591c to
678b74f
Compare
tricknotes
changed the title
Two implementations decided whether Yarn installs an application's dependencies, and they disagreed. `EmberCli::App#yarn_enabled?` read the `yarn` option alone, while `EmberCli::PathSet#yarn?` — which `Shell#install` reaches through `PathSet#yarn` — also treated `yarn_path` as a request for Yarn. The Heroku generator asked the first one. An application configured with `yarn_path` alone therefore installed with Yarn everywhere the gem runs the installer, but got no root `yarn.lock` from `rails generate ember:heroku`, so Heroku's NodeJS buildpack — which picks the package manager by the lockfile it finds — fell back to npm for the same project. `App#yarn?` now delegates to `PathSet#yarn?`, leaving the installer as the single source of truth and the generator as one of its readers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014rAfkAVGVifbaEoTVeT66u
tricknotes
force-pushed
the
unify-yarn-detection
branch
from
September 7, 2026 12:24
678b74f to
4cdbfd0
Compare
tricknotes
marked this pull request as ready for review
September 7, 2026 12:25
This was referenced Sep 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two implementations answered "does Yarn install this application's dependencies?" and they disagreed.
EmberCli::App#yarn_enabled?read theyarnoption alone.EmberCli::PathSet#yarn?— whichShell#installreaches throughPathSet#yarn— also treatedyarn_pathas a request for Yarn.PathSet#path_for_executableresolvesyarn_pathwithout consulting$PATH, andspec/lib/ember_cli/path_set_spec.rbcovers exactly that case.The Heroku generator asked the first one. An application configured with
yarn_pathalone therefore installed with Yarn everywhere the gem runs the installer, but got no rootyarn.lockfromrails generate ember:heroku— so Heroku's NodeJS buildpack, which picks the package manager by the lockfile it finds, fell back to npm for the same project.What changed
EmberCli::PathSet#yarn?is now public and returns a boolean instead of the option value.EmberCli::App#yarn_enabled?is replaced byEmberCli::App#yarn?, delegating toPathSet#yarn?.HerokuGenerator#identify_as_yarn_projectasksEmberCli.any?(&:yarn?).The installer stays the single source of truth; the generator becomes one of its readers.
Compatibility
EmberCli::App#yarn_enabled?is removed rather than aliased — the rename is noted in the CHANGELOG. Its only caller in this repository was the Heroku generator.Behaviour changes for one configuration:
c.app :frontend, yarn_path: "/path/to/yarn"(withoutyarn: true) now gets ayarn.lockfrom the generator. That is the configuration the bug describes.Verification
The one failure is
EmberCli::App#test exits with exit status of 0, which fails onmainin this container too —ember testreportsLauncher Chrome not found, unrelated to this change.New coverage:
PathSet#yarn?foryarn: true,yarn_pathalone, and neither;App#yarn?for the same three plusyarn: false.🤖 Generated with Claude Code
https://claude.ai/code/session_014rAfkAVGVifbaEoTVeT66u
Generated by Claude Code