Skip to content

build: don't put .node_modules/bin on the path#18181

Merged
benesch merged 1 commit intocockroachdb:masterfrom
benesch:no-node-modules-bin
Sep 5, 2017
Merged

build: don't put .node_modules/bin on the path#18181
benesch merged 1 commit intocockroachdb:masterfrom
benesch:no-node-modules-bin

Conversation

@benesch
Copy link
Copy Markdown
Contributor

@benesch benesch commented Sep 2, 2017

Since Yarn 0.25 (yarnpkg/yarn#3310), Yarn transitively symlinks binaries
into node_modules/.bin. (Previously, it would only sync top-level
binaries--i.e., binaries that we directly depend on.) The new behavior
results in adding all sorts of junk to the path--notably, a JavaScript
which reimplementation that shadows the system-provided which. Ew.

Avoid putting node_modules/.bin on the PATH by using full paths to the
binaries within, or using yarn run if within pkg/ui.

@benesch benesch requested review from a team, bdarnell and knz September 2, 2017 16:43
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Thanks, this makes me happy today. 💖

@benesch benesch force-pushed the no-node-modules-bin branch from 3c776d3 to 805dab0 Compare September 3, 2017 04:33
@bdarnell
Copy link
Copy Markdown
Contributor

bdarnell commented Sep 3, 2017

:lgtm:


Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@benesch benesch force-pushed the no-node-modules-bin branch from 805dab0 to 8a30993 Compare September 3, 2017 20:24
Since Yarn 0.25 (yarnpkg/yarn#3310), Yarn transitively symlinks binaries
into node_modules/.bin. (Previously, it would only sync top-level
binaries--i.e., binaries that we directly depend on.) The new behavior
results in adding all sorts of junk to the path--notably, a JavaScript
which reimplementation that shadows the system-provided which. Ew.

Avoid putting node_modules/.bin on the PATH by using full paths to the
binaries within, or using `yarn run` if within pkg/ui.
@benesch benesch force-pushed the no-node-modules-bin branch from 8a30993 to 52b7ccc Compare September 5, 2017 03:29
@benesch benesch merged commit d65ed58 into cockroachdb:master Sep 5, 2017
@benesch benesch deleted the no-node-modules-bin branch September 5, 2017 12:25
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.

4 participants