BUG: Dependencies used in postinstall are unavailable when "flattened" to higher directory.

[ryan-roemer]

Reproduction:

• Create an empty project with the package.json with code provided below.
• npm install
• Expected Behavior: Everything successfully installs
• Actual Behavior: Install fails with error provided below
  • Webpack script is not found
  • Local builder is not found

Diagnosis

Guesses:

• Offhand, most likely candidate is some weirdness with npm v3 tree flattening and git install somehow keeping things nested in an unexpected way
• We may need to "detect" if we are a dependency and if in npm v3, add ../ to NODE_PATH and ../.bin to PATH.

Observations:

• The npm installed packages (victory-animation, victory-label) have lib/, so they're just not hitting the builder run npm:postinstall step. victory-util does not have lib/ (which is expected since it's git installed).
• The npm installed packages (victory-animation, victory-label) have a contained node_modules/builder installation. victory-util does not.

package.json

{
  "name": "victory-error",
  "version": "1.0.0",
  "dependencies": {
    "builder": "~2.4.0",
    "builder-victory-component": "~0.0.9",
    "victory-animation": "^0.0.13",
    "victory-label": "^0.1.8",
    "victory-util": "git+https://github.com/formidablelabs/victory-util#d780bf0"
  },
  "devDependencies": {
    "builder-victory-component-dev": "~0.0.9",
    "chai": "^3.2.0",
    "mocha": "^2.2.5",
    "react": "0.14.x",
    "react-addons-test-utils": "^0.14.0",
    "react-dom": "0.14.x",
    "sinon": "^1.15.4",
    "sinon-chai": "^2.8.0"
  }
}

Errors

Cannot Find Webpack in Archetype

[builder:proc:start] Command: webpack --bail --config node_modules/builder-victory-component/config/webpack/webpack.config.js --colors
module.js:338
    throw err;
    ^

Error: Cannot find module '/Users/rroemer/Desktop/victory-error/node_modules/victory-util/node_modules/builder-victory-component/config/webpack/webpack.config.js'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:286:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at module.exports (/Users/rroemer/Desktop/victory-error/node_modules/webpack/bin/convert-argv.js:80:13)
    at Object.<anonymous> (/Users/rroemer/Desktop/victory-error/node_modules/webpack/bin/webpack.js:39:40)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)

Cannot Find Local Builder

[builder:builder-core:end:28443] Task: run build-dist, Error: Command failed: /bin/sh -c builder run clean-dist && builder run build-dist-min && builder run build-dist-dev
[builder:local-detect] Error importing local builder: Cannot find module '/Users/rroemer/Desktop/victory-error/node_modules/victory-util/node_modules/builder/bin/builder-core.js'
[builder:local-detect] Error importing local builder: Cannot find module '/Users/rroemer/Desktop/victory-error/node_modules/victory-util/node_modules/builder/bin/builder-core.js'

[ryan-roemer]

This is a more generic problem with flattening (in apparently npmv2 or npmv3). Faster repro:

// /Users/rye/scm/fmd/builder-npm3-postinstall-bug-one
{
  "name": "builder-npm3-postinstall-bug-one",
  "version": "0.0.0",
  "scripts": {
    "postinstall": "npm run which-builder",
    "which-builder": "which builder || echo 'ERROR: ONE'"
  },
  "dependencies": {
    "builder": "~2.2.0"
  }
}

// /Users/rye/scm/fmd/builder-npm3-postinstall-bug-two
{
  "name": "builder-npm3-postinstall-bug-two",
  "version": "0.0.0",
  "scripts": {
    "postinstall": "npm run which-builder",
    "which-builder": "which builder || echo 'ERROR: TWO'"
  },
  "dependencies": {
    "builder": "~2.4.0"
  }
}

// /Users/rye/scm/fmd/builder-npm3-postinstall-bug-app
{
  "name": "builder-npm3-postinstall-bug-app",
  "version": "0.0.0",
  "scripts": {
    "postinstall": "npm run which-builder",
    "which-builder": "which builder || echo 'ERROR: APP'"
  },
  "dependencies": {
    "builder": "~2.1.0",
    "builder-npm3-postinstall-one": "file:/Users/rye/scm/fmd/builder-npm3-postinstall-bug-one",
    "builder-npm3-postinstall-two": "file:/Users/rye/scm/fmd/builder-npm3-postinstall-bug-two"
  }
}

Note: That all builder versions are different. Works on both npmv2 and npmv3:

npmv3

// REDOING

npmv2

$ npm install ~/scm/fmd/builder-npm3-postinstall-bug-app/

> which builder || echo 'ERROR: ONE'

/Users/rye/Desktop/builder-npm3-testing/node_modules/builder-npm3-postinstall-bug-app/node_modules/builder-npm3-postinstall-bug-one/node_modules/.bin/builder

> which builder || echo 'ERROR: TWO'

/Users/rye/Desktop/builder-npm3-testing/node_modules/builder-npm3-postinstall-bug-app/node_modules/builder-npm3-postinstall-bug-two/node_modules/.bin/builder

> which builder || echo 'ERROR: APP'

/Users/rye/Desktop/builder-npm3-testing/node_modules/builder-npm3-postinstall-bug-app/node_modules/.bin/builder

So, same output in both cases. Now, let's make all "builder": "~2.4.0" and try again:

npmv3

// REDOING

npmv2

$ npm install ~/scm/fmd/builder-npm3-postinstall-bug-app/

> which builder || echo 'ERROR: ONE'

ERROR: ONE

> which builder || echo 'ERROR: TWO'

ERROR: TWO

> which builder || echo 'ERROR: APP'

/Users/rye/Desktop/builder-npm3-testing/node_modules/builder-npm3-postinstall-bug-app/node_modules/.bin/builder

So the conclusion at this point is that on installs of probably any kind and for npm v2 or v3 we need to detect:

1. Where is builder for a postinstall command? (May be at ../builder -- yuck).
2. What other things have to change to accomodate this?

[boygirl]

maybe archetypes have their own postinstall and a different postinstall for the consuming package? We are already referencing this as builder run npm:postinstall in the consuming packages, so I don't think using non-canonical npm names is so bad. I'm not sure how much this will help the situation though.

[ryan-roemer]

@boygirl -- I'm saying "postinstall cannot rely on anything in dependencies in the worst case". So postinstall cannot use webpack, rimraf, babel, or anything not already part of the Node core library.

[boygirl]

but what if the postinstall for the consuming package expected a path relative to the consuming package node_modules for all those dependencies? They all get installed, and then flattening shouldnt matter. possible?

[exogen]

Our current understanding of the issue is that webpack --bail --config node_modules/builder-victory-component/config/webpack/webpack.config.js --colors fails because it can't find the config, not because it can't find webpack. And it can't find the config because builder-victory-component was flattened into the parent directory's node_modules, so doesn't exist at ./node_modules/builder-victory-component.

So how can we get this config path to work, without resorting to writing all our tasks in JS? A couple proposals, which both rely on preprocessing the script command:

1. Preprocess the command string before it's run, substituting paths using an explicit opt-in syntax.
   • e.g. replace $RESOLVE(builder-victory-component/config/webpack/webpack.config.js) with the result of running require.resolve("builder-victory-component/config/webpack/webpack.config.js")
   • I recommend not attempting to parse the shell command at all. Just do blind substitution on the entire command string, not per-arg.
   • Question: OK, what regex is captured exactly?
     • [$]RESOLVE\([^)]+\). This would capture any character between $RESOLVE( and the next ).
   • Question: What would fail?
     • Filenames with ).
2. Preprocess the command string before it's run, automatically resolving any path that looks like node_modules/foo.
   • e.g. replace node_modules/builder-victory-component with the result of running (pseudo-code) basedir(require.resolve("builder-victory-component/package.json"))
   • I recommend not attempting to parse the shell command at all. Just do blind substitution on the entire command string, not per-arg.
   • Question: OK, what regex is captured exactly?
     • Simplified version: (\./)?node_modules/(NPM_PACKAGE_NAME_REGEX). Capturing only the package name, and not the entire path, is what will allow us to get away with blindly substituting the whole string instead of parsing it. That way, we won't fail on filenames with spaces (for example). Valid npm package names.
   • Question: What would fail?
     • Strings in the command that incidentally contain node_modules/... that that user isn't expecting to be fiddled with.

[ryan-roemer]

New implementation update:

• Have various feature-expand-archetype branches in builder and victory / victory archetype repos.
• Currently working on npm install FormidableLabs/victory-util#feature-expand-archetype

Status: Webpack build is failing with these stats:

• Config: /try-util/node_modules/builder-victory-component/config/webpack/webpack.config.js
• PWD: /try-util/node_modules/victory-util
• __dirname: /try-util/node_modules/builder-victory-component/config/webpack

Failing with:

ModuleParseError: Module parse failed: /Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/victory-util/src/index.js Line 1: Unexpected token
You may need an appropriate loader to handle this file type.
| import * as Collection from "./collection";
| import Helpers from "./helpers";
| import * as Log from "./log";
    at DependenciesBlock.<anonymous> (/Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/webpack/lib/NormalModule.js:113:20)
    at DependenciesBlock.onModuleBuild (/Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/webpack-core/lib/NormalModuleMixin.js:310:10)
    at nextLoader (/Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/webpack-core/lib/NormalModuleMixin.js:275:25)
    at /Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/webpack-core/lib/NormalModuleMixin.js:259:5
    at Storage.finished (/Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/enhanced-resolve/lib/CachedInputFileSystem.js:38:16)
    at /Users/rroemer/Desktop/TEMP_VICTORY/try-util/node_modules/graceful-fs/graceful-fs.js:78:16
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:380:3)

This actually looks right. Investigating more.