From e33079d504dda48e5582f5c6841ed1c4c44c9593 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Thu, 13 Feb 2025 16:16:18 +0800 Subject: [PATCH] fix: correct detect transformed module external request --- packages/core/src/config.ts | 87 +++++++++++++++---- pnpm-lock.yaml | 3 + tests/integration/auto-external/index.test.ts | 33 +++++-- .../module-import-warn/package.json | 1 + .../module-import-warn/rslib.config.ts | 16 ++++ .../module-import-warn/src/index.ts | 12 ++- 6 files changed, 123 insertions(+), 29 deletions(-) diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index ec8140e59..d87b504e6 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -141,6 +141,39 @@ export async function loadConfig({ return { content: content as RslibConfig, filePath: configFilePath }; } +// Match logic is derived from https://github.com/webpack/webpack/blob/94aba382eccf3de1004d235045d4462918dfdbb7/lib/ExternalModuleFactoryPlugin.js#L89-L158 +const handleMatchedExternal = ( + value: string | string[] | boolean | Record, + request: string, +): boolean => { + if (typeof value === 'boolean') { + return value; + } + + if (typeof value === 'string') { + const [first, second] = value.split(' '); + const hasType = !!second; + const _request = second ? second : first; + + // Don't need to warn explicit declared external type. + if (!hasType) { + return request === _request; + } + + return false; + } + + if (Array.isArray(value)) { + return handleMatchedExternal(value[0] ?? '', request); + } + + if (typeof value === 'object') { + return false; + } + + return false; +}; + const composeExternalsWarnConfig = ( format: Format, ...externalsArray: NonNullable['externals'][] @@ -163,18 +196,24 @@ const composeExternalsWarnConfig = ( const matchUserExternals = ( externals: NonNullable['externals'], request: string, - callback: (matched?: true) => void, + callback: (matched: boolean, shouldWarn?: boolean) => void, ) => { + // string if (typeof externals === 'string') { - if (externals === request) { - callback(true); + if (handleMatchedExternal(externals, request)) { + callback(true, true); return; } - } else if (Array.isArray(externals)) { + } + // array + if (Array.isArray(externals)) { let i = 0; const next = () => { let asyncFlag: boolean; - const handleExternalsAndCallback = (matched?: true) => { + const handleExternalsAndCallback = ( + matched: boolean, + shouldWarn?: boolean, + ) => { if (!matched) { if (asyncFlag) { asyncFlag = false; @@ -183,13 +222,13 @@ const composeExternalsWarnConfig = ( return next(); } - callback(matched); + callback(matched, shouldWarn); }; do { asyncFlag = true; if (i >= externals.length) { - return callback(); + return callback(false); } matchUserExternals( externals[i++], @@ -202,37 +241,47 @@ const composeExternalsWarnConfig = ( next(); return; - } else if (externals instanceof RegExp) { + } + // regexp + if (externals instanceof RegExp) { if (externals.test(request)) { - callback(true); + callback(true, true); return; } - } else if (typeof externals === 'function') { + } + // function + else if (typeof externals === 'function') { // TODO: Support function - } else if (typeof externals === 'object') { + } + // object + else if (typeof externals === 'object') { if (Object.prototype.hasOwnProperty.call(externals, request)) { - callback(true); + if (handleMatchedExternal(externals[request]!, request)) { + callback(true, true); + } else { + callback(true); + } return; } } - callback(); + callback(false); }; return { output: { externals: [ ({ request, dependencyType, contextInfo }: any, callback: any) => { - let externalized = false; - const _callback = (matched?: true) => { - if (matched) { - externalized = true; + let shouldWarn = false; + const _callback = (_matched: boolean, _shouldWarn?: boolean) => { + if (_shouldWarn) { + shouldWarn = true; } }; if (contextInfo.issuer && dependencyType === 'commonjs') { matchUserExternals(externals, request, _callback); - if (externalized) { + if (shouldWarn) { logger.warn(composeModuleImportWarn(request)); } } @@ -1449,8 +1498,8 @@ async function composeLibRsbuildConfig( const dtsConfig = await composeDtsConfig(config, dtsExtension); const externalsWarnConfig = composeExternalsWarnConfig( format!, - autoExternalConfig?.output?.externals, userExternalsConfig?.output?.externals, + autoExternalConfig?.output?.externals, ); const minifyConfig = composeMinifyConfig(config); const bannerFooterConfig = composeBannerFooterConfig(banner, footer); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0bdd51bf9..cb1154998 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -549,6 +549,9 @@ importers: tests/integration/auto-external/module-import-warn: dependencies: + lodash: + specifier: ^4.17.21 + version: 4.17.21 react: specifier: ^19.0.0 version: 19.0.0 diff --git a/tests/integration/auto-external/index.test.ts b/tests/integration/auto-external/index.test.ts index 02dc74da7..65b4967b8 100644 --- a/tests/integration/auto-external/index.test.ts +++ b/tests/integration/auto-external/index.test.ts @@ -89,15 +89,30 @@ test('should get warn when use require in ESM', async () => { const { entries } = await buildAndGetResults({ fixturePath }); const logStrings = logs.map((log) => stripAnsi(log)); - expect(entries.esm).toContain( - 'import * as __WEBPACK_EXTERNAL_MODULE_react__ from "react"', - ); - - expect( - logStrings.some((l) => - l.includes(stripAnsi(composeModuleImportWarn('react'))), - ), - ).toBe(true); + const shouldWarn = ['react', 'e2', 'e3', 'e5', 'e6', 'e7']; + const shouldNotWarn = ['e1', 'e4', 'e8', 'lodash/add', 'lodash/drop']; + + for (const item of shouldWarn) { + expect(entries.esm).toContain( + `import * as __WEBPACK_EXTERNAL_MODULE_${item}__ from "${item}"`, + ); + } + + for (const item of shouldWarn) { + expect( + logStrings.some((l) => + l.includes(stripAnsi(composeModuleImportWarn(item))), + ), + ).toBe(true); + } + + for (const item of shouldNotWarn) { + expect( + logStrings.some((l) => + l.includes(stripAnsi(composeModuleImportWarn(item))), + ), + ).toBe(false); + } restore(); }); diff --git a/tests/integration/auto-external/module-import-warn/package.json b/tests/integration/auto-external/module-import-warn/package.json index 13b34dc86..8a61dcb05 100644 --- a/tests/integration/auto-external/module-import-warn/package.json +++ b/tests/integration/auto-external/module-import-warn/package.json @@ -2,6 +2,7 @@ "name": "module-import-warn", "private": true, "dependencies": { + "lodash": "^4.17.21", "react": "^19.0.0" } } diff --git a/tests/integration/auto-external/module-import-warn/rslib.config.ts b/tests/integration/auto-external/module-import-warn/rslib.config.ts index a79eb97f0..438ac421c 100644 --- a/tests/integration/auto-external/module-import-warn/rslib.config.ts +++ b/tests/integration/auto-external/module-import-warn/rslib.config.ts @@ -8,4 +8,20 @@ export default defineConfig({ index: './src/index.ts', }, }, + output: { + externals: [ + { + e1: 'commonjs e1', + e2: 'e2', + e3: true, + e4: ['commonjs e4'], + e5: ['e5'], + 'lodash/add': false, + 'lodash/drop': 'commonjs lodash/drop', + e8: ['module e8'], + }, + /e6/, + 'e7', + ], + }, }); diff --git a/tests/integration/auto-external/module-import-warn/src/index.ts b/tests/integration/auto-external/module-import-warn/src/index.ts index 247b767eb..5b8d3f384 100644 --- a/tests/integration/auto-external/module-import-warn/src/index.ts +++ b/tests/integration/auto-external/module-import-warn/src/index.ts @@ -1,4 +1,14 @@ export const foo = () => { const React = require('react'); - return React.version; + const e1 = require('e1'); + const e2 = require('e2'); + const e3 = require('e3'); + const e4 = require('e4'); + const e5 = require('e5'); + const e6 = require('e6'); + const e7 = require('e7'); + const e8 = require('e8'); + const add = require('lodash/add'); + const drop = require('lodash/drop'); + return React.version + e1 + e2 + e3 + e4 + e5 + e6 + e7 + e8 + add + drop; };