From efc469b6d4a60f8f5aa16527aec0bf6018c0254c Mon Sep 17 00:00:00 2001 From: Navindra Umanee Date: Thu, 18 Jun 2026 20:15:20 -0700 Subject: [PATCH] fix: anchor webpack-config extension source detection to the real src dir The config identifies an extension's own source files (under its `src/` dir) using loose "src" matching: `include: /src/` in index.cjs, `resource.split(path.sep).includes('src')` / `resource.includes('/src/')` in RegisterAsyncChunksPlugin, and `absolutePathToImport.includes('src')` in autoChunkNameLoader. When a project is checked out under a path that itself contains a "src" segment (e.g. ~/src/my-ext), node_modules paths also contain "src", so: the auto-export loader runs on third-party modules (e.g. @babel/runtime helpers) and emits invalid flarum.reg.add(...) (build fails with "Module parse failed"); RegisterAsyncChunksPlugin treats node_modules as extension source and throws "Cannot read properties of undefined (reading 'includes')"; and autoChunkNameLoader produces chunk names that leak the checkout path. Anchor all of these to the extension's actual src directory (path.resolve(process.cwd(), 'src') / this.rootContext) using directory prefix matching, and harden the _source._value access. No-op for checkouts not under a "src" path. Adds regression tests. --- .../src/RegisterAsyncChunksPlugin.cjs | 15 ++- .../src/autoChunkNameLoader.cjs | 8 +- js-packages/webpack-config/src/index.cjs | 4 +- .../tests/fixtures/composer.json | 3 + .../tests/srcPathAnchoring.test.js | 95 +++++++++++++++++++ 5 files changed, 116 insertions(+), 9 deletions(-) create mode 100644 js-packages/webpack-config/tests/fixtures/composer.json create mode 100644 js-packages/webpack-config/tests/srcPathAnchoring.test.js diff --git a/js-packages/webpack-config/src/RegisterAsyncChunksPlugin.cjs b/js-packages/webpack-config/src/RegisterAsyncChunksPlugin.cjs index 118c795e8f..8604d9f3e8 100644 --- a/js-packages/webpack-config/src/RegisterAsyncChunksPlugin.cjs +++ b/js-packages/webpack-config/src/RegisterAsyncChunksPlugin.cjs @@ -41,19 +41,25 @@ class RegisterAsyncChunksPlugin { const chunkModuleMemory = {}; const modulesToCheck = {}; + // The extension's own source directory. Used to identify which modules + // belong to the extension (as opposed to third-party `node_modules`). + // Anchoring to this absolute path avoids false positives when the + // project is checked out under a path that contains a "src" segment. + const srcDir = path.resolve(process.cwd(), 'src') + path.sep; + for (const chunk of chunks) { for (const module of compilation.chunkGraph.getChunkModulesIterable(chunk)) { modulesToCheck[chunk.id] = modulesToCheck[chunk.id] || []; // A normal module. - if (module?.resource && module.resource.split(path.sep).includes('src') && module._source?._value.includes('webpackChunkName: ')) { + if (module?.resource && module.resource.startsWith(srcDir) && module._source?._value?.includes('webpackChunkName: ')) { modulesToCheck[chunk.id].push(module); } // A ConcatenatedModule. if (module?.modules) { module.modules.forEach((module) => { - if (module.resource && module.resource.split(path.sep).includes('src') && module._source?._value.includes('webpackChunkName: ')) { + if (module.resource && module.resource.startsWith(srcDir) && module._source?._value?.includes('webpackChunkName: ')) { modulesToCheck[chunk.id].push(module); } }); @@ -129,13 +135,12 @@ class RegisterAsyncChunksPlugin { // This is a chunk with many modules, we need to register all of them. modules?.forEach((module) => { - if (!module.resource?.includes(`${path.sep}src${path.sep}`)) { + if (!module.resource?.startsWith(srcDir)) { return; } // The path right after the src/ directory, without the extension. - const regPathSep = `\\${path.sep}`; - const urlPath = this.processUrlPath(module.resource.replace(new RegExp(`.*${regPathSep}src${regPathSep}([^.]+)\..+`), '$1')); + const urlPath = this.processUrlPath(module.resource.slice(srcDir.length).replace(/\..+$/, '')); if (!registrableModulesUrlPaths.has(urlPath)) { registrableModulesUrlPaths.set(urlPath, [relevantChunk.id, moduleId, namespace, urlPath]); diff --git a/js-packages/webpack-config/src/autoChunkNameLoader.cjs b/js-packages/webpack-config/src/autoChunkNameLoader.cjs index e5825edf03..59d54782fb 100644 --- a/js-packages/webpack-config/src/autoChunkNameLoader.cjs +++ b/js-packages/webpack-config/src/autoChunkNameLoader.cjs @@ -62,8 +62,12 @@ module.exports = function autoChunkNameLoader(source) { const absolutePathToImport = path.resolve(path.dirname(pathToThisModule), relativePathToImport); let chunkPath = relativePathToImport; - if (absolutePathToImport.includes('src')) { - chunkPath = absolutePathToImport.split(`src${path.sep}`)[1]; + // The chunk name should be the path relative to the extension's `src` + // directory. Anchoring to the absolute `src` path avoids picking up an + // unrelated "src" segment from the project's checkout location. + const srcDir = path.resolve(this.rootContext || process.cwd(), 'src') + path.sep; + if (absolutePathToImport.startsWith(srcDir)) { + chunkPath = absolutePathToImport.slice(srcDir.length); } if (path.sep == '\\') { diff --git a/js-packages/webpack-config/src/index.cjs b/js-packages/webpack-config/src/index.cjs index 6a82738720..f18b96e2f8 100644 --- a/js-packages/webpack-config/src/index.cjs +++ b/js-packages/webpack-config/src/index.cjs @@ -80,11 +80,11 @@ module.exports = function () { module: { rules: [ { - include: /src/, + include: path.resolve(process.cwd(), 'src') + path.sep, loader: path.resolve(__dirname, './autoExportLoader.cjs'), }, { - include: /src/, + include: path.resolve(process.cwd(), 'src') + path.sep, loader: path.resolve(__dirname, './autoChunkNameLoader.cjs'), }, { diff --git a/js-packages/webpack-config/tests/fixtures/composer.json b/js-packages/webpack-config/tests/fixtures/composer.json new file mode 100644 index 0000000000..fcca1947e6 --- /dev/null +++ b/js-packages/webpack-config/tests/fixtures/composer.json @@ -0,0 +1,3 @@ +{ + "name": "acme/my-ext" +} diff --git a/js-packages/webpack-config/tests/srcPathAnchoring.test.js b/js-packages/webpack-config/tests/srcPathAnchoring.test.js new file mode 100644 index 0000000000..9ada1e3814 --- /dev/null +++ b/js-packages/webpack-config/tests/srcPathAnchoring.test.js @@ -0,0 +1,95 @@ +/** + * @jest-environment node + * + * Regression tests for the "checkout path contains a `src` segment" bug. + * + * The config detects an extension's own source files by anchoring to the + * project's `src` directory. Earlier versions matched the substring "src" + * anywhere in a module's absolute path, which broke when the project itself was + * checked out under a path containing a `src` segment (e.g. `~/src/my-ext`): + * + * - the auto-export loader ran on `node_modules` (e.g. `@babel/runtime` + * helpers) and emitted invalid `flarum.reg.add(...)`, failing the build; + * - `autoChunkNameLoader` produced chunk names that leaked the checkout path. + * + * These tests pin the anchored behaviour so it can't regress to loose matching. + */ +import path from 'path'; +import makeConfig from '../src/index.cjs'; +import autoChunkNameLoader from '../src/autoChunkNameLoader.cjs'; + +describe('index.cjs: source loaders are anchored to the project `src` directory', () => { + const srcDir = path.resolve(process.cwd(), 'src') + path.sep; + + // Avoid noise from getEntryPoints() when no forum.ts/admin.ts entry exists. + const config = (() => { + const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); + try { + return makeConfig(); + } finally { + spy.mockRestore(); + } + })(); + + const sourceLoaderRules = config.module.rules.filter( + (rule) => typeof rule.loader === 'string' && /(autoExportLoader|autoChunkNameLoader)\.cjs$/.test(rule.loader) + ); + + test('both source loaders use an absolute `src` path include, not a loose /src/ regex', () => { + expect(sourceLoaderRules).toHaveLength(2); + + for (const rule of sourceLoaderRules) { + expect(rule.include instanceof RegExp).toBe(false); + expect(rule.include).toBe(srcDir); + } + }); + + test('the include matches extension source but not node_modules under an unrelated "src" path', () => { + const include = sourceLoaderRules[0].include; + + // Mirror webpack's own condition matching: a RegExp uses `.test()`, a + // string matches by directory prefix. + const matches = (resource) => (include instanceof RegExp ? include.test(resource) : resource.startsWith(include)); + + // The extension's own source is included. + expect(matches(path.join(process.cwd(), 'src', 'forum', 'index.ts'))).toBe(true); + + // A node_modules file whose absolute path contains an unrelated `src` + // segment must NOT be treated as extension source. A loose /src/ regex + // wrongly matches this — that was the bug. + expect(matches(path.join(path.sep + 'home', 'src', 'proj', 'node_modules', '@babel', 'runtime', 'defineProperty.js'))).toBe(false); + }); +}); + +describe('autoChunkNameLoader: chunk names are relative to the extension `src`', () => { + // Simulate a project checked out under a path that itself contains a `src` + // segment. The extension's own source lives at `/src`. + const root = path.resolve(path.sep + 'home', 'src', 'acme', 'js'); + const resourcePath = path.join(root, 'src', 'forum', 'index.ts'); + + const runLoader = (source) => + autoChunkNameLoader.call( + { + query: { composerPath: path.resolve(__dirname, 'fixtures', 'composer.json') }, + rootContext: root, + resourcePath, + addDependency() {}, + }, + source + ); + + test('internal dynamic import gets a chunk name relative to /src', () => { + const output = runLoader("export function load() {\n return import('./Lazy');\n}\n"); + + // Relative to `/src` -> `forum/Lazy`, NOT polluted by the leading + // `/home/src/...` segment of the checkout path (old output was `acme/js/`). + expect(output).toContain("webpackChunkName: 'forum/Lazy'"); + expect(output).not.toContain('acme/js'); + }); + + test('external (flarum/ and ext:) imports are converted to async module imports', () => { + const output = runLoader("export function load() {\n return import('ext:flarum/tags/forum/components/TagsPage');\n}\n"); + + expect(output).toContain("flarum.reg.asyncModuleImport('ext:flarum/tags/forum/components/TagsPage')"); + }); +});