feat(links): add lifecycle scripts interdependency

Adds the ability for lifecycle scripts of Link nodes to depend
on each other, such that a `prepare` script of linked module A
can make use of files that are a result of a `prepare` script of
a linked package B.

This is important in order to unlock workflows in which a workspace
needs to make use of another workspace but they all have transpilation
steps (very common in Typescript projects) so tracking the order
(and completion) in which the dependencies lifecycle scripts runs
becomes necessary.

Fixes: npm/cli#3034

Author: ruyadorno

lib/arborist/rebuild.js

@@ -249,73 +249,162 @@ module.exports = cls => class Builder extends cls {
     if (!queue.length)
       return
 
+    // helper class to manage script execution
+    class Script {
+      constructor (opts) {
+        const {
+          node,
+          timer,
+          ...runScriptOpts
+        } = opts
+        this.node = node
+        this.timer = timer
+        this.opts = runScriptOpts
+        this.pkg = opts.pkg
+        this.path = opts.path
+        this.env = opts.env
+        this.ref = this.node.target.path
+
+        if (!Script.runPromises)
+          Script.runPromises = new Map()
+      }
+
+      // when the script to be run belongs to a link node, this method
+      // retrieves a list of all other link nodes that are direct dependencies
+      // of the current node, their scripts also have to be part of the current
+      // queue, otherwise it just returns an empty list
+      // this is necessary in order to enable workspaces lifecycle scripts
+      // that depend on each other
+      linkLinkedDeps () {
+        const res = []
+
+        if (!this.node.isLink)
+          return res
+
+        for (const edge of this.node.target.edgesOut.values()) {
+          if (edge.to.isLink &&
+            queue.some(node => node.isLink && node.target.name === edge.name))
+            res.push(edge.to.target.path)
+        }
+
+        return res
+      }
+
+      // use the list of depended scripts from linkLinkedDeps()
+      // and returns a Promise that resolves once all scripts
+      // this current script depends are resolved
+      async linkLinkedDepsResolved () {
+        const resolved = (ref) => {
+          const p = Script.runPromises.get(ref)
+          // polling mechanism used in case the depended script is not
+          // yet set in Script.runPromises, this allow us to keep the
+          // alphabetical order of execution at first and the dependency
+          // mechanism for linked deps is only used when necessary
+          if (!p) {
+            return new Promise(res =>
+              setTimeout(() => res(resolved(ref)), 1))
+          }
+          return p
+        }
+
+        return await Promise.all(
+          this.linkLinkedDeps().map(ref => resolved(ref)))
+      }
+
+      async run () {
+        // in case the current script belongs to a link node that also has
+        // linked nodes dependencies (e.g: workspaces that depend on each other)
+        // then we await for that dependency script to be resolved before
+        // executing the current script
+        await this.linkLinkedDepsResolved()
+
+        // executes the current script using @npmcli/run-script
+        const p = runScript(this.opts)
+
+        // keeps a pool of executing scripts in order to track
+        // inter-dependencies between these scripts
+        Script.runPromises.set(this.ref, p)
+        return p
+      }
+    }
+
     process.emit('time', `build:run:${event}`)
     const stdio = this.options.foregroundScripts ? 'inherit' : 'pipe'
     const limit = this.options.foregroundScripts ? 1 : undefined
-    await promiseCallLimit(queue.map(node => async () => {
-      const {
-        path,
-        integrity,
-        resolved,
-        optional,
-        peer,
-        dev,
-        devOptional,
-        package: pkg,
-        location,
-      } = node.target
-
-      // skip any that we know we'll be deleting
-      if (this[_trashList].has(path))
-        return
-
-      const timer = `build:run:${event}:${location}`
-      process.emit('time', timer)
-      this.log.info('run', pkg._id, event, location, pkg.scripts[event])
-      const env = {
-        npm_package_resolved: resolved,
-        npm_package_integrity: integrity,
-        npm_package_json: resolve(path, 'package.json'),
-        npm_package_optional: boolEnv(optional),
-        npm_package_dev: boolEnv(dev),
-        npm_package_peer: boolEnv(peer),
-        npm_package_dev_optional:
-          boolEnv(devOptional && !dev && !optional),
-      }
-      const runOpts = {
-        event,
-        path,
-        pkg,
-        stdioString: true,
-        stdio,
-        env,
-        scriptShell: this[_scriptShell],
-      }
-      const p = runScript(runOpts).catch(er => {
-        const { code, signal } = er
-        this.log.info('run', pkg._id, event, {code, signal})
-        throw er
-      }).then(({args, code, signal, stdout, stderr}) => {
-        this.scriptsRun.add({
-          pkg,
+    await promiseCallLimit(queue
+      .map(node => {
+        const {
           path,
+          integrity,
+          resolved,
+          optional,
+          peer,
+          dev,
+          devOptional,
+          package: pkg,
+          location,
+        } = node.target
+
+        // skip any that we know we'll be deleting
+        if (this[_trashList].has(path))
+          return
+
+        const timer = `build:run:${event}:${location}`
+        process.emit('time', timer)
+        this.log.info('run', pkg._id, event, location, pkg.scripts[event])
+        const env = {
+          npm_package_resolved: resolved,
+          npm_package_integrity: integrity,
+          npm_package_json: resolve(path, 'package.json'),
+          npm_package_optional: boolEnv(optional),
+          npm_package_dev: boolEnv(dev),
+          npm_package_peer: boolEnv(peer),
+          npm_package_dev_optional:
+            boolEnv(devOptional && !dev && !optional),
+        }
+        return new Script({
+          node,
+          timer,
           event,
-          cmd: args && args[args.length - 1],
+          path,
+          pkg,
+          stdioString: true,
+          stdio,
           env,
-          code,
-          signal,
-          stdout,
-          stderr,
+          scriptShell: this[_scriptShell],
         })
-        this.log.info('run', pkg._id, event, {code, signal})
       })
+      .filter(Boolean)
+      .map(script => async () => {
+        const p = script.run().catch(er => {
+          const { code, signal } = er
+          this.log.info('run', script.pkg._id, event, {code, signal})
+          throw er
+        }).then(({args, code, signal, stdout, stderr}) => {
+          this.scriptsRun.add({
+            pkg: script.pkg,
+            path: script.path,
+            event,
+            cmd: args && args[args.length - 1],
+            env: script.env,
+            code,
+            signal,
+            stdout,
+            stderr,
+          })
+          this.log.info('run', script.pkg._id, event, {code, signal})
+        })
+
+        await (this[_doHandleOptionalFailure]
+          ? this[_handleOptionalFailure](script.node, p)
+          : p)
+
+        process.emit('timeEnd', script.timer)
+      }), limit)
 
-      await (this[_doHandleOptionalFailure]
-        ? this[_handleOptionalFailure](node, p)
-        : p)
+    // release pool refs to scripts promises
+    delete Script.runPromises
 
-      process.emit('timeEnd', timer)
-    }), limit)
     process.emit('timeEnd', `build:run:${event}`)
   }
 

test/arborist/rebuild.js

@@ -713,3 +713,112 @@ t.test('only rebuild for workspace', async t => {
   t.equal(fs.readFileSync(adepTxt, 'utf8'), 'adep', 'adep rebuilt')
   t.throws(() => fs.readFileSync(bdepTxt, 'utf8'), { code: 'ENOENT' }, 'bdep not rebuilt')
 })
+
+t.test('scripts dependencies between Link nodes', async t => {
+  // this scenario is laid out as such: the `prepare` script of each linked
+  // pkg needs the resulting files of its dependency `prepare` script:
+  //
+  // prepare: b -> a -> c
+  // postinstall: e -> a `prepare`
+  //
+  // in order to make sure concurrency is handled properly, the `prepare`
+  // script of "c" takes at least 20ms to complete, while "a" takes at
+  // least 10ms and the "b" script expects to run synchronously
+
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      dependencies: {
+        a: 'file:./packages/a',
+        b: 'file:./packages/b',
+        c: 'file:./packages/c',
+        d: 'file:./packages/d',
+      },
+    }),
+    node_modules: {
+      a: t.fixture('symlink', '../packages/a'),
+      b: t.fixture('symlink', '../packages/b'),
+      c: t.fixture('symlink', '../packages/c'),
+      d: t.fixture('symlink', '../packages/d'),
+      abbrev: {
+        'package.json': JSON.stringify({
+          name: 'abbrev',
+          version: '1.1.1',
+        }),
+      },
+    },
+    packages: {
+      a: {
+        'package.json': JSON.stringify({
+          name: 'a',
+          version: '1.0.0',
+          scripts: {
+            // on prepare script writes a `index.js` file containing:
+            // module.exports = require("c")
+            // this is a slow script though that sleeps for
+            // at least 10ms prior to writing that file
+            prepare: "node -e \"setTimeout(() => { require('fs').writeFileSync(require('path').resolve('index.js'), 'module.exports = require(function c(){}.name);') }, 10)\"",
+          },
+          dependencies: {
+            c: '^1.0.0',
+          },
+        }),
+      },
+      b: {
+        'package.json': JSON.stringify({
+          name: 'b',
+          version: '1.0.0',
+          scripts: {
+            // on prepare script requires `./node_modules/a/index.js` which
+            // is a dependency of this workspace but with the caveat that this
+            // file is only build during the `prepare` script of "a"
+            prepare: "node -p \"require('a')\"",
+          },
+          dependencies: {
+            a: '^1.0.0',
+            abbrev: '^1.0.0',
+          },
+        }),
+      },
+      c: {
+        'package.json': JSON.stringify({
+          name: 'c',
+          version: '1.0.0',
+          scripts: {
+            // on prepare script writes a `index.js` file containing:
+            // module.exports = "HELLO"
+            // this is an even slower slower script that sleeps for
+            // at least 20ms prior to writing that file
+            prepare: "node -e \"setTimeout(() => { require('fs').writeFileSync(require('path').resolve('index.js'), 'module.exports = function HELLO() {}.name;') }, 20)\"",
+          },
+        }),
+      },
+      d: {
+        'package.json': JSON.stringify({
+          name: 'd',
+          version: '1.0.0',
+        }),
+      },
+      e: {
+        'package.json': JSON.stringify({
+          name: 'd',
+          version: '1.0.0',
+          scripts: {
+            // on postinstall script requires `./node_modules/a/index.js` which
+            // is a dependency of this workspace but with the caveat that this
+            // file is only build during the `prepare` script of "a"
+            // although different lifecycle scripts do not hold refs to
+            // depending on each other, all `prepare` scripts should already
+            // have been resolved by the time we run `postinstall` scripts
+            postinstall: "node -p \"require('a')\"",
+          },
+          dependencies: {
+            a: '^1.0.0',
+          },
+        }),
+      },
+    },
+  })
+
+  const arb = newArb({ path })
+  await arb.rebuild()
+})