Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

feat: npm workspaces #50

Closed
wants to merge 1 commit into from
Closed

Conversation

ruyadorno
Copy link
Contributor

@ruyadorno ruyadorno commented Feb 24, 2020

Overview

Introduces support to workspaces; adding ability to build an ideal tree that links defined workspaces, storing and reading lockfiles that contains workspaces info and also reifying installation trees properly symlinking nested workspaces into place.

✏️ Changes

  • lib/arborist/build-ideal-tree.js Creates trees that take into account workspaces definitions
  • lib/arborist/load-virtual.js Added ability to read workspaces definitions
  • lib/edge.js Added check that a workspace edges are symlinks
  • lib/node.js Added workspaces setter that will properly create the edgesOut pointing to nested workspaces
  • lib/shrinkwrap.js Add workspaces info to lockfile

🔗 References

Introduces support to workspaces; adding ability to build an ideal tree
that links defined workspaces, storing and reading lockfiles that
contains workspaces info and also reifying installation trees properly
symlinking nested workspaces into place.

Handling of the config definitions is done via @npmcli/map-workspaces
module added.

refs:

- https://github.com/npm/rfcs/blob/ea2d3024e6e149cd8c6366ed18373c9a566b1124/accepted/0026-workspaces.md
- https://www.npmjs.com/package/@npmcli/map-workspaces
- npm/rfcs#103
@ruyadorno ruyadorno changed the title Workspaces wip feat: npm workspaces Apr 30, 2020
@ruyadorno ruyadorno marked this pull request as ready for review April 30, 2020 15:33
@@ -15,6 +16,7 @@ const resolveLinks = Symbol('resolveLinks')
const assignParentage = Symbol('assignParentage')
const loadNode = Symbol('loadVirtualNode')
const loadLink = Symbol('loadVirtualLink')
const loadWorkspaces = Symbol('loadWorkspaces')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just noticing this now, not relevant to this PR, but we should make loadVirtual underscore its symbols like the other mixin classes.

Copy link
Contributor

@isaacs isaacs left a comment

Choose a reason for hiding this comment

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

One minor nit that I'm pretty sure the hasWorkspaces option to the Node class in buildIdealTree isn't relevant. (Tests pass without that line?)

Other than that, this is great! For such a significant feature, it's impressive how little code actually needed to be changed or added to make it happen.

LGTM!

@@ -234,9 +237,19 @@ module.exports = cls => class IdealTreeBuilder extends Tracker(Virtual(Actual(cl
optional: false,
global: this[_global],
legacyPeerDeps: this.legacyPeerDeps,
hasWorkspaces: !!pkg.workspaces,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see where this field is used, is it a leftover from something that changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yeah def a leftover, will clean it up 😊

@isaacs isaacs closed this in f024962 May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants