Diagnostic for undeclared external dependencies in library builds (#6564

)
parcel-bundler · Jul 9, 2021 · 278cfe8 · 278cfe8
1 parent db75d9c
commit 278cfe8
Show file tree

Hide file tree

Showing 23 changed files with 322 additions and 61 deletions.
diff --git a/packages/core/integration-tests/test/integration/formats/cjs-type-module/package.json b/packages/core/integration-tests/test/integration/formats/cjs-type-module/package.json
@@ -2,5 +2,8 @@
   "name": "cjs-type-module",
   "private": true,
   "type": "module",
-  "main": "dist/index.cjs"
+  "main": "dist/index.cjs",
+  "dependencies": {
+    "lodash": "*"
+  }
 }
diff --git a/...e/integration-tests/test/integration/formats/commonjs-destructuring-browsers/package.json b/...e/integration-tests/test/integration/formats/commonjs-destructuring-browsers/package.json
@@ -2,5 +2,8 @@
   "name": "commonjs-destructuring-browsers",
   "private": true,
   "browser": "dist/index.js",
-  "browserslist": [">= 0.25%"]
+  "browserslist": [">= 0.25%"],
+  "dependencies": {
+    "lodash": "*"
+  }
 }
diff --git a/.../core/integration-tests/test/integration/formats/commonjs-destructuring-node/package.json b/.../core/integration-tests/test/integration/formats/commonjs-destructuring-node/package.json
@@ -4,5 +4,8 @@
   "main": "dist/index.js",
   "engines": {
     "node": ">= 4.x"
+  },
+  "dependencies": {
+    "lodash": "*"
   }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-esm/package.json b/packages/core/integration-tests/test/integration/formats/commonjs-esm/package.json
@@ -1,5 +1,8 @@
 {
   "name": "commonjs-esm",
   "private": true,
-  "module": "dist/index.js"
+  "module": "dist/index.js",
+  "dependencies": {
+    "lodash": "*"
+  }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-external/package.json b/packages/core/integration-tests/test/integration/formats/commonjs-external/package.json
@@ -4,5 +4,8 @@
   "main": "dist/index.js",
   "engines": {
     "node": ">= 10.x"
+  },
+  "dependencies": {
+    "lodash": "*"
   }
 }
diff --git a/...ages/core/integration-tests/test/integration/formats/commonjs-live-externals/package.json b/...ages/core/integration-tests/test/integration/formats/commonjs-live-externals/package.json
@@ -2,7 +2,10 @@
   "main": "dist/test.js",
   "targets": {
   	"main": {
-	  "includeNodeModules": false
-	}
+      "includeNodeModules": false
+    }
+  },
+  "dependencies": {
+    "external": "*"
   }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/commonjs-require/package.json b/packages/core/integration-tests/test/integration/formats/commonjs-require/package.json
@@ -6,5 +6,8 @@
     "main": {
       "context": "node"
     }
+  },
+  "dependencies": {
+    "lodash": "*"
   }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/esm-conflict/package.json b/packages/core/integration-tests/test/integration/formats/esm-conflict/package.json
@@ -1,5 +1,8 @@
 {
   "name": "esm-conflict",
   "private": true,
-  "module": "dist/index.js"
+  "module": "dist/index.js",
+  "dependencies": {
+    "foo": "*"
+  }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/esm-external/package.json b/packages/core/integration-tests/test/integration/formats/esm-external/package.json
@@ -5,5 +5,8 @@
   "sideEffects": false,
   "engines": {
     "node": ">= 10.x"
+  },
+  "dependencies": {
+    "lodash": "*"
   }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/esm-import-shadow/package.json b/packages/core/integration-tests/test/integration/formats/esm-import-shadow/package.json
@@ -4,5 +4,8 @@
     "module": {
       "includeNodeModules": false
     }
+  },
+  "dependencies": {
+    "b": "*"
   }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/esm-mjs/package.json b/packages/core/integration-tests/test/integration/formats/esm-mjs/package.json
@@ -1,5 +1,8 @@
 {
   "name": "esm-mjs",
   "private": true,
-  "main": "dist/index.mjs"
+  "main": "dist/index.mjs",
+  "dependencies": {
+    "lodash": "*"
+  }
 }
diff --git a/packages/core/integration-tests/test/integration/formats/esm-type-module/package.json b/packages/core/integration-tests/test/integration/formats/esm-type-module/package.json
@@ -2,5 +2,8 @@
   "name": "esm-type-module",
   "private": true,
   "type": "module",
-  "main": "dist/index.js"
+  "main": "dist/index.js",
+  "dependencies": {
+    "lodash": "*"
+  }
 }
diff --git a/...egration-tests/test/integration/scope-hoisting/es6/import-namespace-external/package.json b/...egration-tests/test/integration/scope-hoisting/es6/import-namespace-external/package.json
@@ -5,5 +5,8 @@
             "isLibrary": true,
             "context": "node"
         }
+    },
+    "dependencies": {
+        "lodash": "*"
     }
 }
diff --git a/packages/core/integration-tests/test/integration/ts-types/externals/package.json b/packages/core/integration-tests/test/integration/ts-types/externals/package.json
@@ -2,5 +2,10 @@
   "name": "ts-types-externals",
   "private": true,
   "main": "dist/main.js",
-  "types": "dist/types.d.ts"
+  "types": "dist/types.d.ts",
+  "dependencies": {
+    "react": "*",
+    "external": "*",
+    "other-external": "*"
+  }
 }
diff --git a/packages/core/integration-tests/test/integration/undeclared-external/helpers.js b/packages/core/integration-tests/test/integration/undeclared-external/helpers.js
@@ -0,0 +1 @@
+class Test {}
diff --git a/packages/core/integration-tests/test/integration/undeclared-external/index.js b/packages/core/integration-tests/test/integration/undeclared-external/index.js
@@ -0,0 +1,3 @@
+import {add} from 'lodash';
+
+console.log(add(2, 3));
diff --git a/packages/core/integration-tests/test/integration/undeclared-external/package.json b/packages/core/integration-tests/test/integration/undeclared-external/package.json
@@ -0,0 +1,6 @@
+{
+  "name": "undeclared-external",
+  "main": "dist/out.js",
+  "browserslist": ">= 0.25%",
+  "dependencies": {}
+}
diff --git a/packages/core/integration-tests/test/integration/undeclared-external/yarn.lock b/packages/core/integration-tests/test/integration/undeclared-external/yarn.lock
diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js
@@ -14,7 +14,7 @@ import {
   outputFS,
   inputFS,
 } from '@parcel/test-utils';
-import {makeDeferredWithPromise} from '@parcel/utils';
+import {makeDeferredWithPromise, normalizePath} from '@parcel/utils';
 import vm from 'vm';
 
 describe('javascript', function() {
@@ -4344,4 +4344,152 @@ describe('javascript', function() {
       },
     ]);
   });
+
+  it('should error on undeclared external dependencies for libraries', async function() {
+    let fixture = path.join(
+      __dirname,
+      'integration/undeclared-external/index.js',
+    );
+    let pkg = path.join(
+      __dirname,
+      'integration/undeclared-external/package.json',
+    );
+    await assert.rejects(
+      () =>
+        bundle(fixture, {
+          mode: 'production',
+          defaultTargetOptions: {
+            shouldOptimize: false,
+          },
+        }),
+      {
+        name: 'BuildError',
+        diagnostics: [
+          {
+            message: "Failed to resolve 'lodash' from './index.js'",
+            origin: '@parcel/core',
+            codeFrames: [
+              {
+                code: await inputFS.readFile(fixture, 'utf8'),
+                filePath: fixture,
+                codeHighlights: [
+                  {
+                    start: {
+                      line: 1,
+                      column: 19,
+                    },
+                    end: {
+                      line: 1,
+                      column: 26,
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+          {
+            message:
+              'External dependency "lodash" is not declared in package.json.',
+            origin: '@parcel/resolver-default',
+            codeFrames: [
+              {
+                code: await inputFS.readFile(pkg, 'utf8'),
+                filePath: pkg,
+                language: 'json',
+                codeHighlights: [
+                  {
+                    message: undefined,
+                    start: {
+                      line: 5,
+                      column: 3,
+                    },
+                    end: {
+                      line: 5,
+                      column: 16,
+                    },
+                  },
+                ],
+              },
+            ],
+            hints: ['Add "lodash" as a dependency.'],
+          },
+        ],
+      },
+    );
+  });
+
+  it('should error on undeclared helpers dependency for libraries', async function() {
+    let fixture = path.join(
+      __dirname,
+      'integration/undeclared-external/helpers.js',
+    );
+    let pkg = path.join(
+      __dirname,
+      'integration/undeclared-external/package.json',
+    );
+    await assert.rejects(
+      () =>
+        bundle(fixture, {
+          mode: 'production',
+          defaultTargetOptions: {
+            shouldOptimize: false,
+          },
+        }),
+      {
+        name: 'BuildError',
+        diagnostics: [
+          {
+            message: `Failed to resolve '@swc/helpers' from '${normalizePath(
+              require.resolve('@parcel/transformer-js/src/JSTransformer.js'),
+            )}'`,
+            origin: '@parcel/core',
+            codeFrames: [
+              {
+                code: await inputFS.readFile(fixture, 'utf8'),
+                filePath: fixture,
+                codeHighlights: [
+                  {
+                    start: {
+                      line: 1,
+                      column: 1,
+                    },
+                    end: {
+                      line: 1,
+                      column: 0,
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+          {
+            message:
+              'External dependency "@swc/helpers" is not declared in package.json.',
+            origin: '@parcel/resolver-default',
+            codeFrames: [
+              {
+                code: await inputFS.readFile(pkg, 'utf8'),
+                filePath: pkg,
+                language: 'json',
+                codeHighlights: [
+                  {
+                    message: undefined,
+                    start: {
+                      line: 5,
+                      column: 3,
+                    },
+                    end: {
+                      line: 5,
+                      column: 16,
+                    },
+                  },
+                ],
+              },
+            ],
+            hints: ['Add "@swc/helpers" as a dependency.'],
+          },
+        ],
+      },
+    );
+  });
 });
diff --git a/packages/resolvers/default/src/DefaultResolver.js b/packages/resolvers/default/src/DefaultResolver.js
@@ -27,6 +27,7 @@ export default (new Resolver({
       isURL: dependency.specifierType === 'url',
       parent: dependency.resolveFrom,
       env: dependency.env,
+      sourcePath: dependency.sourcePath,
     });
   },
 }): Resolver);
diff --git a/packages/transformers/js/src/JSTransformer.js b/packages/transformers/js/src/JSTransformer.js
@@ -619,17 +619,23 @@ export default (new Transformer({
           };
         }
 
+        // Always bundle helpers, even with includeNodeModules: false, except if this is a library.
+        let isHelper = dep.is_helper && !dep.specifier.endsWith('/jsx-runtime');
+        if (isHelper && !asset.env.isLibrary) {
+          env = {
+            ...env,
+            includeNodeModules: true,
+          };
+        }
+
         asset.addDependency({
           specifier: dep.specifier,
           specifierType: dep.kind === 'Require' ? 'commonjs' : 'esm',
           loc: convertLoc(dep.loc),
           priority: dep.kind === 'DynamicImport' ? 'lazy' : 'sync',
           isOptional: dep.is_optional,
           meta,
-          resolveFrom:
-            dep.is_helper && !dep.specifier.endsWith('/jsx-runtime')
-              ? __filename
-              : undefined,
+          resolveFrom: isHelper ? __filename : undefined,
           env,
         });
       }