feat(check:policy): Add policy to check for correct dependency types (#…

…12724)

Some libraries we depend on, like typescript and eslint, recommend using
tilde dependencies instead of caret because they do introduce breaking
changes in minor versions. This change adds a policy handler to enforce
it.

More discussion at
<#9966 (comment)>
and in <#9982>.

build-tools/packages/build-tools/package.json

@@ -86,7 +86,7 @@
     "@types/json5": "^0.0.30",
     "@types/lodash.isequal": "^4.5.5",
     "@types/lodash.merge": "^4.6.6",
-    "@types/minimatch": "3.0.5",
+    "@types/minimatch": "^3.0.5",
     "@types/mocha": "^9.0.0",
     "@types/node": "^14.18.0",
     "@types/rimraf": "^2.0.3",

build-tools/packages/build-tools/src/common/fluidRepo.ts

@@ -7,15 +7,27 @@ import * as path from "path";
 import { getPackageManifest } from "./fluidUtils";
 import { Logger, defaultLogger } from "./logging";
 import { MonoRepo, MonoRepoKind, isMonoRepoKind } from "./monoRepo";
-import { Package, Packages } from "./npmPackage";
+import { Package, Packages, ScriptDependencies } from "./npmPackage";
 import { ExecAsyncResult } from "./utils";
 
 export interface IPackageManifest {
     repoPackages: {
         [name: string]: IFluidRepoPackageEntry;
     };
-    additionalLockfilePaths?: string[];
+    buildDependencies?: {
+        merge?: {
+            [key: string]: ScriptDependencies;
+        };
+    };
     generatorName?: string;
+    policy?: PolicyConfig;
+}
+
+export interface PolicyConfig {
+    additionalLockfilePaths?: string[];
+    dependencies?: {
+        requireTilde?: string[];
+    };
 }
 
 export interface IFluidRepoPackage {

build-tools/packages/build-tools/src/common/npmPackage.ts

@@ -10,6 +10,7 @@ import * as path from "path";
 import sortPackageJson from "sort-package-json";
 
 import { options } from "../fluidBuild/options";
+import { IPackageManifest } from "./fluidRepo";
 import { defaultLogger } from "./logging";
 import { MonoRepo, MonoRepoKind } from "./monoRepo";
 import {
@@ -34,7 +35,7 @@ interface IPerson {
     url: string;
 }
 
-interface IPackage {
+export interface IPackage {
     name: string;
     version: string;
     private: boolean;
@@ -64,13 +65,7 @@ interface IPackage {
     cpu: string[];
     [key: string]: any;
 
-    fluidBuild?: {
-        buildDependencies: {
-            merge?: {
-                [key: string]: ScriptDependencies;
-            };
-        };
-    };
+    fluidBuild?: IPackageManifest;
 }
 
 export class Package {

build-tools/packages/build-tools/src/repoPolicyCheck/handlers/lockfiles.ts

@@ -21,7 +21,7 @@ const getKnownPaths = (manifest: IPackageManifest) => {
         _knownPaths = ["."];
 
         // Add additional paths from the manifest
-        _knownPaths.push(...(manifest.additionalLockfilePaths ?? []));
+        _knownPaths.push(...(manifest.policy?.additionalLockfilePaths ?? []));
 
         // Add paths to known monorepos and packages
         const vals = Object.values(manifest.repoPackages).filter(

build-tools/packages/build-tools/src/repoPolicyCheck/handlers/npmPackages.ts

@@ -13,6 +13,9 @@ import * as readline from "readline";
 import replace from "replace-in-file";
 import sortPackageJson from "sort-package-json";
 
+import { IPackageManifest } from "../../common/fluidRepo";
+import { getPackageManifest } from "../../common/fluidUtils";
+import { IPackage } from "../../common/npmPackage";
 import { Handler, readFile, writeFile } from "../common";
 
 const licenseId = "MIT";
@@ -27,6 +30,18 @@ or logos must follow Microsoft's [Trademark & Brand Guidelines](https://www.micr
 Use of Microsoft trademarks or logos in modified versions of this project must not cause confusion or imply Microsoft sponsorship.
 `;
 
+/**
+ * An array of dependencies that should always use tilde dependency ranges instead of caret.
+ */
+let _tildeDependencies: string[] | undefined;
+
+const getTildeDependencies = (manifest: IPackageManifest | undefined) => {
+    if (_tildeDependencies === undefined) {
+        _tildeDependencies = manifest?.policy?.dependencies?.requireTilde ?? [];
+    }
+    return _tildeDependencies;
+};
+
 // Some of our package scopes definitely should publish, and others should never publish.  If they should never
 // publish, we want to add the "private": true flag to their package.json to prevent publishing, and conversely if
 // they should always publish we should ensure the "private": true flag is not present.  There is then a third
@@ -481,9 +496,9 @@ export const handlers: Handler[] = [
     {
         name: "npm-package-json-lint",
         match,
-        handler: (file) => {
+        handler: (file, root) => {
             let jsonStr: string;
-            let json;
+            let json: IPackage;
             try {
                 jsonStr = readFile(file);
                 json = JSON.parse(jsonStr);
@@ -493,6 +508,25 @@ export const handlers: Handler[] = [
 
             const ret: string[] = [];
 
+            const { dependencies, devDependencies } = json;
+            const manifest = getPackageManifest(root);
+            const tildeDependencies = getTildeDependencies(manifest);
+            if (dependencies !== undefined) {
+                for (const [dep, ver] of Object.entries(json.dependencies)) {
+                    if (tildeDependencies.includes(dep) && !ver.startsWith("~")) {
+                        ret.push(`Dependencies on ${dep} must use tilde (~) dependencies.`);
+                    }
+                }
+            }
+
+            if (devDependencies !== undefined) {
+                for (const [dep, ver] of Object.entries(json.devDependencies)) {
+                    if (tildeDependencies.includes(dep) && !ver.startsWith("~")) {
+                        ret.push(`devDependencies on ${dep} must use tilde (~) dependencies.`);
+                    }
+                }
+            }
+
             const { valid, validationResults } = runNpmJsonLint(json, file);
 
             if (!valid) {

experimental/dds/tree-graphql/package.json

@@ -59,7 +59,7 @@
     "eslint": "~8.6.0",
     "mocha": "^10.0.0",
     "nyc": "^15.0.0",
-    "prettier": "^2.3.1",
+    "prettier": "~2.3.1",
     "rimraf": "^2.6.2",
     "typescript": "~4.5.5"
   },

experimental/dds/tree/package.json

@@ -84,7 +84,7 @@
     "eslint": "~8.6.0",
     "mocha": "^10.0.0",
     "nyc": "^15.0.0",
-    "prettier": "^2.3.1",
+    "prettier": "~2.3.1",
     "random-js": "^1.0.8",
     "rimraf": "^2.6.2",
     "typescript": "~4.5.5"

package.json

@@ -162,13 +162,30 @@
         ]
       }
     },
-    "additionalLockfilePaths": [
-      "common/build/build-common",
-      "common/build/eslint-config-fluid",
-      "docs",
-      "server/gitrest",
-      "server/historian",
-      "tools/telemetry-generator"
-    ]
+    "policy": {
+      "dependencies": {
+        "requireTilde": [
+          "@typescript-eslint/eslint-plugin",
+          "@typescript-eslint/parser",
+          "eslint-config-prettier",
+          "eslint-plugin-eslint-comments",
+          "eslint-plugin-import",
+          "eslint-plugin-unicorn",
+          "eslint-plugin-unused-imports",
+          "eslint",
+          "prettier",
+          "typescript",
+          "webpack-dev-server"
+        ]
+      },
+      "additionalLockfilePaths": [
+        "common/build/build-common",
+        "common/build/eslint-config-fluid",
+        "docs",
+        "server/gitrest",
+        "server/historian",
+        "tools/telemetry-generator"
+      ]
+    }
   }
 }

packages/dds/tree/package.json

@@ -107,7 +107,7 @@
     "good-fences": "^1.1.1",
     "mocha": "^10.0.0",
     "nyc": "^15.0.0",
-    "prettier": "^2.3.1",
+    "prettier": "~2.3.1",
     "rimraf": "^2.6.2",
     "source-map-support": "^0.5.16",
     "typescript": "~4.5.5",