fix: validate non-peerDependencies at build time (#687)

Author: dherges

integration/samples.sh

@@ -13,9 +13,13 @@ do
     elif [ -f $P_JSON ]; then
         echo "Building sample ${P_JSON}..."
         node dist/cli/main.js -p ${P_JSON}
-    else
+    elif [ -f $P ]; then
         echo "Building sample ${P}..."
-        node dist/cli/main.js -p ${P}
+        if [ $P='failures' ]; then
+            node dist/cli/main.js -p ${P} || true
+        else
+            node dist/cli/main.js -p ${P}
+        fi
     fi
     echo "Built."
 done

integration/samples/failures/index.ts

@@ -0,0 +1 @@
+export const PUBLISHING_NON_PEER_DEPENDENCIES = 'should fail';

integration/samples/failures/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "non-peer-deps",
+  "description": "https://github.com/dherges/ng-packagr/pull/687",
+  "dependencies": {
+    "foobar": "1.0.0"
+  },
+  "ngPackage": {
+    "dest": "./dist",
+    "lib": {
+      "entryFile": "index.ts"
+    }
+  }
+}

integration/samples/failures/specs/non-peer-deps.ts

@@ -0,0 +1,12 @@
+import { expect } from 'chai';
+import * as fs from 'fs-extra';
+import * as path from 'path';
+
+describe(`Failure Builds`, () => {
+  describe(`non-peer-deps`, () => {
+    it(`should have no build output`, () => {
+      const exists = fs.existsSync(path.resolve(__dirname, 'dist'));
+      expect(exists).to.be.false;
+    });
+  });
+});

package.json

@@ -122,11 +122,14 @@
     "publish:ci": "yarn prerelease && yarn postrelease",
     "integration:samples": "integration/samples.sh",
     "integration:samples:dev": "ts-node --project src/tsconfig.packagr.json ./integration/samples.dev.ts",
-    "integration:specs": "cross-env TS_NODE_PROJECT=integration/tsconfig.specs.json mocha --require ts-node/register integration/samples/*/specs/**/*.ts",
+    "integration:specs":
+      "cross-env TS_NODE_PROJECT=integration/tsconfig.specs.json mocha --require ts-node/register \"integration/samples/*/specs/**/*.ts\"",
     "integration:consumers": "integration/consumers.sh",
     "integration:consumers:ngc": "ngc -p integration/consumers/tsc/tsconfig.json",
-    "test:specs": "cross-env TS_NODE_PROJECT=src/tsconfig.specs.json mocha --require ts-node/register \"src/**/*.spec.ts\"",
-    "test": "yarn build && yarn test:specs && yarn integration:samples && yarn integration:specs && yarn integration:consumers",
+    "test:specs":
+      "cross-env TS_NODE_PROJECT=src/tsconfig.specs.json mocha --require ts-node/register \"src/**/*.spec.ts\"",
+    "test":
+      "yarn build && yarn test:specs && yarn integration:samples && yarn integration:specs && yarn integration:consumers",
     "commitmsg": "commitlint -e",
     "precommit": "pretty-quick --staged",
     "gh-pages": "gh-pages -d docs/ghpages"

src/lib/ng-package-format/package.ts

@@ -70,6 +70,15 @@ export class NgPackage {
     return this.primary.$get('keepLifecycleScripts') === true;
   }
 
+  public get whitelistedNonPeerDependencies(): string[] {
+    // XX: default array values from JSON schema not recognized
+    const defValue = ['tslib'];
+    const value = (this.primary.$get('whitelistedNonPeerDependencies') as string[]) || defValue;
+
+    // Always append 'tslib' and dedupe
+    return value.concat('tslib').filter((value, index, self) => self.indexOf(value) === index);
+  }
+
   private absolutePathFromPrimary(key: string) {
     return path.resolve(this.basePath, this.primary.$get(key));
   }

src/lib/ng-v5/entry-point/write-package.transform.ts

@@ -5,6 +5,7 @@ import { NgEntryPoint } from '../../ng-package-format/entry-point';
 import { NgPackage } from '../../ng-package-format/package';
 import { ensureUnixPath } from '../../util/path';
 import { copyFiles } from '../../util/copy';
+import { rimraf } from '../../util/rimraf';
 import * as log from '../../util/log';
 import { isEntryPointInProgress } from '../nodes';
 
@@ -78,11 +79,16 @@ export async function writePackageJson(
     }
   }
 
-  packageJson.name = entryPoint.moduleId;
-
-  // keep the dist package.json clean
-  // this will not throw if ngPackage field does not exist
-  delete packageJson.ngPackage;
+  // Verify non-peerDependencies as they can easily lead to duplicated installs or version conflicts
+  // in the node_modules folder of an application
+  const whitelist = pkg.whitelistedNonPeerDependencies.map(value => new RegExp(value));
+  try {
+    checkNonPeerDependencies(packageJson, 'dependencies', whitelist);
+    checkNonPeerDependencies(packageJson, 'devDependencies', whitelist);
+  } catch (e) {
+    await rimraf(entryPoint.destinationPath);
+    throw e;
+  }
 
   // Removes scripts from package.json after build
   if (pkg.keepLifecycleScripts !== true) {
@@ -94,6 +100,11 @@ export async function writePackageJson(
     );
   }
 
+  // keep the dist package.json clean
+  // this will not throw if ngPackage field does not exist
+  delete packageJson.ngPackage;
+  packageJson.name = entryPoint.moduleId;
+
   // `outputJson()` creates intermediate directories, if they do not exist
   // -- https://github.com/jprichardson/node-fs-extra/blob/master/docs/outputJson.md
   await fs.outputJson(path.resolve(entryPoint.destinationPath, 'package.json'), packageJson, { spaces: 2 });
@@ -111,3 +122,18 @@ export async function copyJavaScriptBundles(stageDir: string, destDir: string):
 export async function copyTypingsAndMetadata(from: string, to: string): Promise<void> {
   await copyFiles(`${from}/**/*.{d.ts,metadata.json}`, to);
 }
+
+function checkNonPeerDependencies(packageJson: { [key: string]: any }, property: string, whitelist: RegExp[]) {
+  if (packageJson[property]) {
+    Object.keys(packageJson[property]).forEach(dep => {
+      if (whitelist.find(regex => regex.test(dep))) {
+        log.debug(`Dependency ${dep} is whitelisted in '${property}'`);
+      } else {
+        log.warn(
+          `Distributing npm packages with '${property}' is not recommended. Please consider adding ${dep} to 'peerDepenencies' or remove it from '${property}'.`
+        );
+        throw new Error(`Dependency ${dep} must be explicitly whitelisted.`);
+      }
+    });
+  }
+}

src/ng-package.schema.json

@@ -31,6 +31,15 @@
       "type": "string",
       "default": ".ng_pkg_build"
     },
+    "whitelistedNonPeerDependencies": {
+      "description":
+        "A list of dependencies that are allowed in the 'dependendencies' and 'devDependencies' section of package.json. Values in the list are regular expressions matched against npm package names.",
+      "type": "array",
+      "items": {
+        "type": "string"
+      },
+      "default": ["tslib"]
+    },
     "lib": {
       "description": "Description of the library's entry point.",
       "type": "object",
@@ -83,7 +92,7 @@
           "default": "none"
         },
         "sassIncludePaths": {
-          "description": "DEPRECATED: Please use styleIncludePaths",
+          "description": "DEPRECATED: Please use styleIncludePaths. sassIncludePaths will be removed in v3!",
           "type": "array",
           "items": {
             "type": "string"