Skip to content

Fix commands when used in a workspace#4583

Closed
jgoz wants to merge 8 commits intoyarnpkg:masterfrom
jgoz:fix-commands-in-workspace
Closed

Fix commands when used in a workspace#4583
jgoz wants to merge 8 commits intoyarnpkg:masterfrom
jgoz:fix-commands-in-workspace

Conversation

@jgoz
Copy link
Copy Markdown
Contributor

@jgoz jgoz commented Sep 29, 2017

Summary
Fixes #4334
Fixes #4328
Fixes #4348
Closes #4584

Previously, remove, outdated and several related commands would fail when used inside of a workspace package, either directly or via yarn workspaces <pkg> <cmd> [args]. These failures were caused by incorrect lockfile resolution, specifically using config.cwd rather than config.lockfileFolder.

Also fixed:

  • When using remove inside a workspace, reinstall would not clean up extraneous packages.
  • When using outdated, upgrade or upgrade-interactive inside of a workspace root or child, the wrong dependencies were often listed and the lockfile was being saved incorrectly after completing an upgrade
  • Adding a dev dependency to the workspace root would remove all workspace child dependencies from the lockfile (this PR replaces Preserve workspace packages when adding to the root #4584)

Test plan

  • Added relevant unit tests
  • Existing tests pass
  • Manually tested against repro project

@buildsize
Copy link
Copy Markdown

buildsize bot commented Sep 29, 2017

This change will increase the build size from 9.83 MB to 9.84 MB, an increase of 5.74 KB (0%)

File name Previous Size New Size Change
yarn-[version].noarch.rpm 848.46 KB 848.82 KB 371 bytes (0%)
yarn-[version].js 3.74 MB 3.75 MB 2.27 KB (0%)
yarn-legacy-[version].js 3.79 MB 3.8 MB 2.31 KB (0%)
yarn-v[version].tar.gz 854.18 KB 854.56 KB 385 bytes (0%)
yarn_[version]all.deb 645.25 KB 645.67 KB 430 bytes (0%)

@BYK BYK self-assigned this Sep 29, 2017
@arcanis arcanis assigned arcanis and unassigned BYK Sep 29, 2017
}
// look for extraneous packages in workspaces too
if (this.config.lockfileFolder !== this.config.cwd) {
await this._readRegistryFolders(this.config.cwd, possibleExtraneous, scopedPaths);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be a while loop that would iterate on each workspace?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that would make sense. Do you see any value including cwd in addition to lockfileFolder and all of the workspaces?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it's necessary, all the node_modules should be guaranteed to be in these folders.

@jgoz jgoz force-pushed the fix-commands-in-workspace branch from 2bb9f5b to a2baddb Compare September 29, 2017 22:26
@jgoz jgoz changed the title Fix outdated & remove when used in a workspace Fix commands when used in a workspace Sep 29, 2017
@jgoz
Copy link
Copy Markdown
Contributor Author

jgoz commented Sep 29, 2017

@arcanis Please note that the scope of this PR increased somewhat as I continued testing commands that could not be executed previously. I have updated the description accordingly.

In order to properly test the now-available commands, my other fix from #4584 had to be included here. @kaylieEB, I would greatly appreciate your insight on this PR because it deviates from the approach you took in #4278.

@arcanis
Copy link
Copy Markdown
Member

arcanis commented Oct 5, 2017

Closing because of #4630

@arcanis arcanis closed this Oct 5, 2017
@jgoz jgoz deleted the fix-commands-in-workspace branch October 17, 2017 23:22
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.

Adding root dependency removes workspace packages from lockfile "yarn remove" inside workspace fails yarn outdated not working w/workspaces

3 participants