feat!: disable autoExternal in bundleless mode and only redirect re…

…quest not in node_modules (#624)
web-infra-dev · Dec 27, 2024 · b6b21b3 · b6b21b3
1 parent b1cffee
commit b6b21b3
Show file tree

Hide file tree

Showing 17 changed files with 138 additions and 57 deletions.
diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts
@@ -250,12 +250,18 @@ const getAutoExternalDefaultValue = (
 };
 
 export const composeAutoExternalConfig = (options: {
+  bundle: boolean;
   format: Format;
   autoExternal?: AutoExternal;
   pkgJson?: PkgJson;
   userExternals?: NonNullable<EnvironmentConfig['output']>['externals'];
 }): EnvironmentConfig => {
-  const { format, pkgJson, userExternals } = options;
+  const { bundle, format, pkgJson, userExternals } = options;
+
+  // If bundle is false, autoExternal will be disabled
+  if (bundle === false) {
+    return {};
+  }
 
   const autoExternal = getAutoExternalDefaultValue(
     format,
@@ -1020,23 +1026,33 @@ const composeBundlelessExternalConfig = (
 
               if (jsRedirectPath) {
                 try {
+                  // use resolver to resolve the request
                   resolvedRequest = await resolver(context, resolvedRequest);
-                  resolvedRequest = normalizeSlash(
-                    path.relative(
-                      path.dirname(contextInfo.issuer),
-                      resolvedRequest,
-                    ),
-                  );
-                  // Requests that fall through here cannot be matched by any other externals config ahead.
-                  // Treat all these requests as relative import of source code. Node.js won't add the
-                  // leading './' to the relative path resolved by `path.relative`. So add manually it here.
-                  if (resolvedRequest[0] !== '.') {
-                    resolvedRequest = `./${resolvedRequest}`;
+
+                  // only handle the request that is not in node_modules
+                  if (!resolvedRequest.includes('node_modules')) {
+                    resolvedRequest = normalizeSlash(
+                      path.relative(
+                        path.dirname(contextInfo.issuer),
+                        resolvedRequest,
+                      ),
+                    );
+                    // Requests that fall through here cannot be matched by any other externals config ahead.
+                    // Treat all these requests as relative import of source code. Node.js won't add the
+                    // leading './' to the relative path resolved by `path.relative`. So add manually it here.
+                    if (resolvedRequest[0] !== '.') {
+                      resolvedRequest = `./${resolvedRequest}`;
+                    }
+                  } else {
+                    // NOTE: If request is a phantom dependency, which means it can be resolved but not specified in dependencies or peerDependencies in package.json, the output will be incorrect to use when the package is published
+                    // return the original request instead of the resolved request
+                    return callback(undefined, request);
                   }
                 } catch (e) {
+                  // catch error when request can not be resolved by resolver
                   // e.g. A react component library importing and using 'react' but while not defining
                   // it in devDependencies and peerDependencies. Preserve 'react' as-is if so.
-                  logger.warn(
+                  logger.debug(
                     `Failed to resolve module ${color.green(`"${resolvedRequest}"`)} from ${color.green(contextInfo.issuer)}. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.`,
                   );
                 }
@@ -1271,6 +1287,7 @@ async function composeLibRsbuildConfig(
   } = composeTargetConfig(config.output?.target, format!);
   const syntaxConfig = composeSyntaxConfig(target, config?.syntax);
   const autoExternalConfig = composeAutoExternalConfig({
+    bundle,
     format: format!,
     autoExternal,
     pkgJson,

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/tests/integration/auto-external/bundle-false/package.json b/tests/integration/auto-external/bundle-false/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "auto-external-bundle-false-test",
+  "private": true,
+  "devDependencies": {
+    "ora": "8.1.1",
+    "react": "^19.0.0"
+  }
+}
diff --git a/tests/integration/auto-external/bundle-false/rslib.config.ts b/tests/integration/auto-external/bundle-false/rslib.config.ts
@@ -0,0 +1,18 @@
+import { defineConfig } from '@rslib/core';
+import { generateBundleCjsConfig, generateBundleEsmConfig } from 'test-helper';
+
+export default defineConfig({
+  lib: [
+    generateBundleEsmConfig({
+      bundle: false,
+    }),
+    generateBundleCjsConfig({
+      bundle: false,
+    }),
+  ],
+  source: {
+    entry: {
+      index: ['./src/**'],
+    },
+  },
+});
diff --git a/tests/integration/auto-external/bundle-false/src/index.ts b/tests/integration/auto-external/bundle-false/src/index.ts
@@ -0,0 +1,7 @@
+import type { oraPromise } from 'ora';
+import React from 'react';
+
+export type { oraPromise };
+export const foo = () => {
+  return React.version;
+};
diff --git a/tests/integration/auto-external/bundle-false/tsconfig.json b/tests/integration/auto-external/bundle-false/tsconfig.json
@@ -0,0 +1,7 @@
+{
+  "extends": "@rslib/tsconfig/base",
+  "compilerOptions": {
+    "baseUrl": "./"
+  },
+  "include": ["src"]
+}
diff --git a/tests/integration/auto-external/index.test.ts b/tests/integration/auto-external/index.test.ts
@@ -40,6 +40,19 @@ test('auto external sub path should works', async () => {
   );
 });
 
+test('auto external should be disabled when bundle is false', async () => {
+  const fixturePath = join(__dirname, 'bundle-false');
+  const { js } = await buildAndGetResults({ fixturePath, type: 'all' });
+
+  expect(Object.values(js.contents.esm)[0]).toContain(
+    'import * as __WEBPACK_EXTERNAL_MODULE_react__ from "react"',
+  );
+
+  expect(Object.values(js.contents.cjs)[0]).toContain(
+    'const external_react_namespaceObject = require("react");',
+  );
+});
+
 test('auto external false should works', async () => {
   const fixturePath = join(__dirname, 'false');
   const { js, dts } = await buildAndGetResults({ fixturePath, type: 'all' });
@@ -49,7 +62,7 @@ test('auto external false should works', async () => {
   );
 
   expect(js.entries.cjs).not.toContain(
-    'var external_react_namespaceObject = require("react");',
+    'const external_react_namespaceObject = require("react");',
   );
 
   // dts should bundled

diff --git a/tests/integration/redirect/js-not-resolve/src/index.js b/tests/integration/redirect/js-not-resolve/src/index.js
@@ -1,5 +1,10 @@
+// can not be resolved
 import lodash from 'lodash';
+// can be resolved but not specified -- phantom dependency
+import prettier from 'prettier';
 import bar from './bar.js';
 import foo from './foo';
 
+console.log('prettier: ', prettier);
+
 export default lodash.toUpper(foo + bar);
diff --git a/tests/integration/redirect/js.test.ts b/tests/integration/redirect/js.test.ts
@@ -21,9 +21,11 @@ test('redirect.js default', async () => {
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_index_js__ from "./bar/index.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
     import * as __WEBPACK_EXTERNAL_MODULE__baz_js__ from "./baz.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__baz_js__.baz);
     export { src_rslib_entry_ as default };
     "
@@ -44,11 +46,13 @@ test('redirect.js.path false', async () => {
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar__ from "@/bar";
     import * as __WEBPACK_EXTERNAL_MODULE__foo__ from "@/foo";
     import * as __WEBPACK_EXTERNAL_MODULE__baz__ from "~/baz";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_js__ from "./bar.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__bar_js__.bar + __WEBPACK_EXTERNAL_MODULE__foo__.foo + __WEBPACK_EXTERNAL_MODULE__bar__.bar + __WEBPACK_EXTERNAL_MODULE__baz__.baz);
     export { src_rslib_entry_ as default };
     "
@@ -67,11 +71,13 @@ test('redirect.js.path with user override externals', async () => {
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__others_bar_index_js__ from "./others/bar/index.js";
     import * as __WEBPACK_EXTERNAL_MODULE__others_foo_js__ from "./others/foo.js";
     import * as __WEBPACK_EXTERNAL_MODULE__baz_js__ from "./baz.js";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_index_js__ from "./bar/index.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__others_foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__others_bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__baz_js__.baz);
     export { src_rslib_entry_ as default };
     "
@@ -98,11 +104,13 @@ test('redirect.js.path with user override alias', async () => {
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__others_bar_index_js__ from "./others/bar/index.js";
     import * as __WEBPACK_EXTERNAL_MODULE__others_foo_js__ from "./others/foo.js";
     import * as __WEBPACK_EXTERNAL_MODULE__baz_js__ from "./baz.js";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_index_js__ from "./bar/index.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__others_foo_js__.foo + __WEBPACK_EXTERNAL_MODULE__others_bar_index_js__.bar + __WEBPACK_EXTERNAL_MODULE__baz_js__.baz);
     export { src_rslib_entry_ as default };
     "
@@ -124,9 +132,11 @@ test('redirect.js.extension: false', async () => {
   );
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_index_ts__ from "./bar/index.ts";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_ts__ from "./foo.ts";
     import * as __WEBPACK_EXTERNAL_MODULE__baz_ts__ from "./baz.ts";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_ts__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_ts__.bar + __WEBPACK_EXTERNAL_MODULE__foo_ts__.foo + __WEBPACK_EXTERNAL_MODULE__bar_index_ts__.bar + __WEBPACK_EXTERNAL_MODULE__baz_ts__.baz);
     export { src_rslib_entry_ as default };
     "

diff --git a/tests/integration/redirect/js/src/index.ts b/tests/integration/redirect/js/src/index.ts
@@ -1,9 +1,14 @@
+// can not be resolved
 import lodash from 'lodash';
+// can be resolved but not specified -- phantom dependency
+import prettier from 'prettier';
 
 import { bar as bar2 } from '@/bar';
 import { foo as foo2 } from '@/foo';
 import { baz } from '~/baz';
 import { bar } from './bar';
 import { foo } from './foo';
 
+console.log('prettier: ', prettier);
+
 export default lodash.toUpper(foo + bar + foo2 + bar2 + baz);
diff --git a/tests/integration/redirect/jsNotResolved.test.ts b/tests/integration/redirect/jsNotResolved.test.ts
@@ -1,38 +1,23 @@
 import path from 'node:path';
-import stripAnsi from 'strip-ansi';
-import { buildAndGetResults, proxyConsole, queryContent } from 'test-helper';
+import { buildAndGetResults, queryContent } from 'test-helper';
 import { expect, test } from 'vitest';
 
 test('redirect.js default', async () => {
   const fixturePath = path.resolve(__dirname, './js-not-resolve');
-  const { logs } = proxyConsole();
   const contents = (await buildAndGetResults({ fixturePath, lib: ['esm0'] }))
     .contents;
 
-  const logStrings = logs
-    .map((log) => stripAnsi(log))
-    .filter((log) => log.startsWith('warn'))
-    .sort();
-
-  expect(logStrings).toMatchInlineSnapshot(
-    `
-    [
-      "warn    Failed to resolve module "./bar.js" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-      "warn    Failed to resolve module "./foo" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-      "warn    Failed to resolve module "lodash" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-    ]
-  `,
-  );
-
   const { content: indexContent } = queryContent(
     contents.esm0!,
     /esm\/index\.js/,
   );
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_js__ from "./bar.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__["default"] + __WEBPACK_EXTERNAL_MODULE__bar_js__["default"]);
     export { src_rslib_entry_ as default };
     "
@@ -41,25 +26,20 @@ test('redirect.js default', async () => {
 
 test('redirect.js.path false', async () => {
   const fixturePath = path.resolve(__dirname, './js-not-resolve');
-  const { logs } = proxyConsole();
   const contents = (await buildAndGetResults({ fixturePath, lib: ['esm1'] }))
     .contents;
 
-  const logStrings = logs
-    .map((log) => stripAnsi(log))
-    .filter((log) => log.startsWith('warn'));
-
-  expect(logStrings.length).toBe(0);
-
   const { content: indexContent } = queryContent(
     contents.esm1!,
     /esm\/index\.js/,
   );
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_js__ from "./bar.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo_js__ from "./foo.js";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo_js__["default"] + __WEBPACK_EXTERNAL_MODULE__bar_js__["default"]);
     export { src_rslib_entry_ as default };
     "
@@ -68,34 +48,20 @@ test('redirect.js.path false', async () => {
 
 test('redirect.js.extension: false', async () => {
   const fixturePath = path.resolve(__dirname, './js-not-resolve');
-  const { logs } = proxyConsole();
   const contents = (await buildAndGetResults({ fixturePath, lib: ['esm2'] }))
     .contents;
 
-  const logStrings = logs
-    .map((log) => stripAnsi(log))
-    .filter((log) => log.startsWith('warn'))
-    .sort();
-
-  expect(logStrings).toMatchInlineSnapshot(
-    `
-    [
-      "warn    Failed to resolve module "./bar.js" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-      "warn    Failed to resolve module "./foo" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-      "warn    Failed to resolve module "lodash" from <ROOT>/tests/integration/redirect/js-not-resolve/src/index.js. If it's an npm package, consider adding it to dependencies or peerDependencies in package.json to make it externalized.",
-    ]
-  `,
-  );
-
   const { content: indexContent } = queryContent(
     contents.esm2!,
     /esm\/index\.js/,
   );
 
   expect(indexContent).toMatchInlineSnapshot(`
     "import * as __WEBPACK_EXTERNAL_MODULE_lodash__ from "lodash";
+    import * as __WEBPACK_EXTERNAL_MODULE_prettier__ from "prettier";
     import * as __WEBPACK_EXTERNAL_MODULE__bar_js__ from "./bar.js";
     import * as __WEBPACK_EXTERNAL_MODULE__foo__ from "./foo";
+    console.log('prettier: ', __WEBPACK_EXTERNAL_MODULE_prettier__["default"]);
     const src_rslib_entry_ = __WEBPACK_EXTERNAL_MODULE_lodash__["default"].toUpper(__WEBPACK_EXTERNAL_MODULE__foo__["default"] + __WEBPACK_EXTERNAL_MODULE__bar_js__["default"]);
     export { src_rslib_entry_ as default };
     "

diff --git a/website/docs/en/config/lib/auto-external.mdx b/website/docs/en/config/lib/auto-external.mdx
@@ -4,6 +4,12 @@ overviewHeaders: [2, 3]
 
 # lib.autoExternal
 
+:::info
+
+`autoExternal` is a specific configuration for bundle mode. It will not take effect in bundleless mode (set [lib.bundle](/config/lib/bundle) to `false`) since deps will not be bundled in bundleless mode.
+
+:::
+
 - **Type:**
 
 ```ts

diff --git a/website/docs/en/config/lib/redirect.mdx b/website/docs/en/config/lib/redirect.mdx
@@ -6,7 +6,7 @@ overviewHeaders: [2, 3]
 
 :::info
 
-Redirect is the unique configuration for bundleless mode (set [lib.bundle](/config/lib/bundle) to `false`). It will not take effect in bundle mode where all output files are packaged into a single file, eliminating the need for import path redirection.
+`redirect` is the unique configuration for bundleless mode (set [lib.bundle](/config/lib/bundle) to `false`). It will not take effect in bundle mode where all output files are packaged into a single file, eliminating the need for import path redirection.
 
 As bundleless mode is still under development, additional redirect configurations will be introduced in the future.
 

diff --git a/website/docs/en/guide/advanced/third-party-deps.mdx b/website/docs/en/guide/advanced/third-party-deps.mdx
@@ -1,5 +1,7 @@
 # Handle Third-Party Dependencies
 
+This section introduces how to handle third-party dependencies in bundle mode.
+
 Generally, third-party dependencies required by a project can be installed via the `install` command in the package manager. After the third-party dependencies are successfully installed, they will generally appear under `dependencies` and `devDependencies` in the project `package.json`.
 
 ```json title="package.json"
-Original file line number
+Diff line change
@@ Expand Up / @@ -4,6 +4,12 @@ overviewHeaders: [2, 3] @@
     # lib.autoExternal
+    :::info
+    `autoExternal` is a specific configuration for bundle mode. It will not take effect in bundleless mode (set [lib.bundle](/config/lib/bundle) to `false`) since deps will not be bundled in bundleless mode.
+    :::
     - **Type:**
     ```ts
@@ Expand Down @@