Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions js-packages/webpack-config/src/RegisterAsyncChunksPlugin.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -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]);
Expand Down
8 changes: 6 additions & 2 deletions js-packages/webpack-config/src/autoChunkNameLoader.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 == '\\') {
Expand Down
4 changes: 2 additions & 2 deletions js-packages/webpack-config/src/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
},
{
Expand Down
3 changes: 3 additions & 0 deletions js-packages/webpack-config/tests/fixtures/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "acme/my-ext"
}
95 changes: 95 additions & 0 deletions js-packages/webpack-config/tests/srcPathAnchoring.test.js
Original file line number Diff line number Diff line change
@@ -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 `<root>/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 <root>/src', () => {
const output = runLoader("export function load() {\n return import('./Lazy');\n}\n");

// Relative to `<root>/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')");
});
});