Install `node_modules` to application directory

The original design decision to install `node_modules` to a central
location outside the application directory was for a purported
development use case not yet realized. It has led to confusion and
unintended failures, and is just generally weird.

Let's right this wrong before it fossilizes into debt, like a hysterical
raisin in the sun.

Bug: T204591
Change-Id: I2e4797833471596b10b6b4a2705055050ca98ac6

2 files changed, 17 insertions, 32 deletions

diff --git a/config/node.go b/config/node.go
index 438e715..a6a22ea 100644
--- a/config/node.go
+++ b/config/node.go
@@ -2,7 +2,6 @@ package config
 
 import (
 	"gerrit.wikimedia.org/r/blubber/build"
-	"path"
 )
 
 // NodeConfig holds configuration fields related to the Node environment and
@@ -27,53 +26,47 @@ func (nc *NodeConfig) Merge(nc2 NodeConfig) {
 }
 
 // InstructionsForPhase injects instructions into the build related to Node
-// dependency installation and setting of the NODE_ENV, NODE_PATH, and PATH
-// environment variables.
+// dependency installation and setting of the NODE_ENV.
 //
 // PhasePreInstall
 //
 // Installs Node package dependencies declared in requirements files into the
-// shared library directory (/opt/lib). Only production related packages are
-// install if NodeConfig.Env is set to "production" in which case `npm dedupe`
-// is also run. Installing dependencies during the build.PhasePreInstall phase
-// allows a compiler implementation (e.g. Docker) to produce cache-efficient
-// output so only changes to package.json will invalidate these steps of the
-// image build.
+// application directory. Only production related packages are install if
+// NodeConfig.Env is set to "production" in which case `npm dedupe` is also
+// run. Installing dependencies during the build.PhasePreInstall phase allows
+// a compiler implementation (e.g. Docker) to produce cache-efficient output
+// so only changes to package.json will invalidate these steps of the image
+// build.
 //
 // PhasePostInstall
 //
-// Injects build.Env instructions for NODE_ENV, NODE_PATH, and PATH, setting
-// the environment according to the configuration, ensuring that packages
-// installed during build.PhasePreInstall are found by Node, and that any
-// installed binaries are found by shells.
+// Injects build.Env instructions for NODE_ENV, setting the environment
+// according to the configuration.
 //
 func (nc NodeConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
 	switch phase {
 	case build.PhasePreInstall:
 		if len(nc.Requirements) > 0 {
 			npmInstall := build.RunAll{[]build.Run{
-				{"cd", []string{LocalLibPrefix}},
 				{"npm install", []string{}},
 			}}
 
 			if nc.Env == "production" {
-				npmInstall.Runs[1].Arguments = []string{"--production"}
+				npmInstall.Runs[0].Arguments = []string{"--production"}
 				npmInstall.Runs = append(npmInstall.Runs,
 					build.Run{"npm dedupe", []string{}},
 				)
 			}
 
 			return append(
-				build.SyncFiles(nc.Requirements, LocalLibPrefix),
+				build.SyncFiles(nc.Requirements, "."),
 				npmInstall,
 			)
 		}
 	case build.PhasePostInstall:
 		if nc.Env != "" || len(nc.Requirements) > 0 {
 			return []build.Instruction{build.Env{map[string]string{
-				"NODE_ENV":  nc.Env,
-				"NODE_PATH": path.Join(LocalLibPrefix, "node_modules"),
-				"PATH":      path.Join(LocalLibPrefix, "node_modules", ".bin") + ":${PATH}",
+				"NODE_ENV": nc.Env,
 			}}}
 		}
 	}
diff --git a/config/node_test.go b/config/node_test.go
index 0cb48fa..ef29ee5 100644
--- a/config/node_test.go
+++ b/config/node_test.go
@@ -69,9 +69,8 @@ func TestNodeConfigInstructionsNonProduction(t *testing.T) {
 	t.Run("PhasePreInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Copy{[]string{"package.json"}, "/opt/lib/"},
+				build.Copy{[]string{"package.json"}, "./"},
 				build.RunAll{[]build.Run{
-					{"cd", []string{"/opt/lib"}},
 					{"npm install", []string{}},
 				}},
 			},
@@ -83,9 +82,7 @@ func TestNodeConfigInstructionsNonProduction(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
 				build.Env{map[string]string{
-					"NODE_ENV":  "foo",
-					"NODE_PATH": "/opt/lib/node_modules",
-					"PATH":      "/opt/lib/node_modules/.bin:${PATH}",
+					"NODE_ENV": "foo",
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),
@@ -107,9 +104,8 @@ func TestNodeConfigInstructionsProduction(t *testing.T) {
 	t.Run("PhasePreInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Copy{[]string{"package.json", "package-lock.json"}, "/opt/lib/"},
+				build.Copy{[]string{"package.json", "package-lock.json"}, "./"},
 				build.RunAll{[]build.Run{
-					{"cd", []string{"/opt/lib"}},
 					{"npm install", []string{"--production"}},
 					{"npm dedupe", []string{}},
 				}},
@@ -122,9 +118,7 @@ func TestNodeConfigInstructionsProduction(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
 				build.Env{map[string]string{
-					"NODE_ENV":  "production",
-					"NODE_PATH": "/opt/lib/node_modules",
-					"PATH":      "/opt/lib/node_modules/.bin:${PATH}",
+					"NODE_ENV": "production",
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),
@@ -151,9 +145,7 @@ func TestNodeConfigInstructionsEnvironmentOnly(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
 				build.Env{map[string]string{
-					"NODE_ENV":  "production",
-					"NODE_PATH": "/opt/lib/node_modules",
-					"PATH":      "/opt/lib/node_modules/.bin:${PATH}",
+					"NODE_ENV": "production",
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),