From 768103a55b51a759c5d9482939bb48a65364ab16 Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 17 Oct 2026 13:43:46 +0100 Subject: [PATCH 1/6] fix(upgrade): read TypeScript that exports from an ambient module --- .../twopointoh/frontend/admin-extender.ts | 2 +- src/steps/upgrade/twopointoh/frontend/misc.ts | 2 +- src/utils/ast.ts | 5 +- src/utils/babel-ts.ts | 17 +++++ test/steps/upgrade/admin-extender.test.ts | 69 +++++++++++++++++++ test/steps/upgrade/format.test.ts | 35 ++++++++++ test/steps/upgrade/initializer-checks.test.ts | 26 +++++++ 7 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/utils/babel-ts.ts create mode 100644 test/steps/upgrade/format.test.ts diff --git a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts index a967ee7..944d57a 100644 --- a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts +++ b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts @@ -4,9 +4,9 @@ import globby from 'globby'; import chalk from 'chalk'; import traverse, { Binding, NodePath } from '@babel/traverse'; import * as t from '@babel/types'; -import { parse } from 'recast/parsers/babel-ts'; import { BaseUpgradeStep, GitCommit, Replacement } from '../base'; import { formatCode } from '../../../../utils/ast'; +import { parse } from '../../../../utils/babel-ts'; import { extensionId } from '../../../../utils/composer'; const TODO = '@TODO: register this with Extend.Admin in admin/extend instead'; diff --git a/src/steps/upgrade/twopointoh/frontend/misc.ts b/src/steps/upgrade/twopointoh/frontend/misc.ts index 356f48e..2e30aeb 100644 --- a/src/steps/upgrade/twopointoh/frontend/misc.ts +++ b/src/steps/upgrade/twopointoh/frontend/misc.ts @@ -4,8 +4,8 @@ import * as t from '@babel/types'; import traverse from '@babel/traverse'; import fs from 'fs'; import path from 'path'; -import { parse } from 'recast/parsers/babel-ts'; import { getFunctionName } from '../../../../utils/ast'; +import { parse } from '../../../../utils/babel-ts'; import { extensionId } from '../../../../utils/composer'; /** The bundled extensions' IDs, which are their initializers' names in 2.0. */ diff --git a/src/utils/ast.ts b/src/utils/ast.ts index 12271d4..bda92a8 100644 --- a/src/utils/ast.ts +++ b/src/utils/ast.ts @@ -6,17 +6,18 @@ import prettierConfig from '@flarum/prettier-config/prettierrc.json'; // import { parse } from '@babel/parser'; // import generate from '@babel/generator'; import * as recast from 'recast'; +import * as babelTs from './babel-ts'; export function parseCode(code: string): t.File { return recast.parse(code, { - parser: require('recast/parsers/babel-ts'), + parser: babelTs, }); } export async function generateCode(ast: t.File, extenders = false): Promise { // @ts-ignore const generatedCode = recast.print(ast, { - parser: require('recast/parsers/babel-ts'), + parser: babelTs, }).code; // Format the code diff --git a/src/utils/babel-ts.ts b/src/utils/babel-ts.ts new file mode 100644 index 0000000..5607eea --- /dev/null +++ b/src/utils/babel-ts.ts @@ -0,0 +1,17 @@ +import { parse as babelParse, ParseResult } from '@babel/parser'; +import * as t from '@babel/types'; +import getBabelOptions from 'recast/parsers/_babel_options'; + +/** + * Recast's babel-ts parser, which also accepts an export of a name it can't see + * declared, as Prettier does. Babel reports one for every `export { name }` in + * an ambient `declare module` block, which is valid TypeScript. The upgrade + * rewrites code rather than checking it: the extension's own build does that. + */ +export function parse(source: string): ParseResult { + const options = getBabelOptions(); + + options.plugins.push('jsx', 'typescript'); + + return babelParse(source, { ...options, allowUndeclaredExports: true }); +} diff --git a/test/steps/upgrade/admin-extender.test.ts b/test/steps/upgrade/admin-extender.test.ts index e3981f9..38140c4 100644 --- a/test/steps/upgrade/admin-extender.test.ts +++ b/test/steps/upgrade/admin-extender.test.ts @@ -77,6 +77,75 @@ export default [ }); }); + it('from an index that exports from an ambient module', async () => { + // Valid TypeScript, which Babel reports as an undeclared export unless it + // is told not to check. + const { index, written } = await upgrade({ + 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; + +declare module 'acme-shims' { + const shim: string; + export { shim }; +} + +app.initializers.add('acme-interstellar', () => { + app.extensionData.for('acme-interstellar').registerSetting({ setting: 'acme-interstellar.fuel', type: 'text' }); +}); +`, + }); + + expect(index).toBe( + await format(`export { default as extend } from './extend'; + +declare module 'acme-shims' { + const shim: string; + export { shim }; +} +`) + ); + + expect(written).toEqual({ + 'js/src/admin/extend.ts': await format(`import Extend from 'flarum/common/extenders'; + +export default [new Extend.Admin().setting(() => ({ setting: 'acme-interstellar.fuel', type: 'text' }))]; +`), + }); + }); + + it('into an admin/extend that exports from an ambient module', async () => { + const { written } = await upgrade({ + 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; + +export { default as extend } from './extend'; + +app.initializers.add('acme-interstellar', () => { + app.extensionData.for('acme-interstellar').registerSetting({ setting: 'acme-interstellar.fuel', type: 'text' }); +}); +`, + 'js/src/admin/extend.ts': `import Extend from 'flarum/common/extenders'; + +declare module 'acme-shims' { + const shim: string; + export { shim }; +} + +export default []; +`, + }); + + expect(written).toEqual({ + 'js/src/admin/extend.ts': await format(`import Extend from 'flarum/common/extenders'; + +declare module 'acme-shims' { + const shim: string; + export { shim }; +} + +export default [new Extend.Admin().setting(() => ({ setting: 'acme-interstellar.fuel', type: 'text' }))]; +`), + }); + }); + it('leaving an initializer that still has other work in place', async () => { const { index } = await upgrade({ 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; diff --git a/test/steps/upgrade/format.test.ts b/test/steps/upgrade/format.test.ts new file mode 100644 index 0000000..0d38ae6 --- /dev/null +++ b/test/steps/upgrade/format.test.ts @@ -0,0 +1,35 @@ +import FormatCode from '../../../src/steps/upgrade/twopointoh/frontend/format'; + +async function upgrade(code: string, file = 'js/src/forum/index.ts'): Promise { + const step = new FormatCode({} as any, '/extension', 1, 1, false); + const result = await step.applyReplacements(file, code, step.advancedContent(file, code)); + + return result.updated as string; +} + +describe('the formatting step', () => { + it('formats a declaration file that exports from an ambient module', async () => { + // As fof/discussion-language's 1.x typings have it. Valid TypeScript, but + // Babel reports the export as undeclared unless it is told not to check. + const source = `declare module '@fof-follow-tags' { + import type Component from 'flarum/common/Component'; + const components: { + SubscriptionModal: typeof Component & { prototype: any }; + }; + export { components }; +} +`; + + expect(await upgrade(source, 'js/src/@types/missing-modules.d.ts')).toBe(source); + }); + + it('still fails a file that is not valid code', () => { + // Only the undeclared export check is off. Any other error Babel finds + // still stops the step, rather than it rewriting code it misread. + const step = new FormatCode({} as any, '/extension', 1, 1, false); + + expect(() => step.advancedContent('js/src/forum/index.ts', 'const a = 1;\nconst a = 2;\n')).toThrow( + "Could not parse: Identifier 'a' has already been declared." + ); + }); +}); diff --git a/test/steps/upgrade/initializer-checks.test.ts b/test/steps/upgrade/initializer-checks.test.ts index 129c10b..38b5334 100644 --- a/test/steps/upgrade/initializer-checks.test.ts +++ b/test/steps/upgrade/initializer-checks.test.ts @@ -44,6 +44,32 @@ if (app.initializers.has('flarum-tags')) { ).toBe( await format(`import app from 'flarum/forum/app'; +if ('flarum-tags' in flarum.extensions) { + console.log('tags'); +} +`) + ); + }); + + it('reads a file that exports from an ambient module', async () => { + // Valid TypeScript, which Babel reports as an undeclared export unless it + // is told not to check. + expect( + await upgrade(`declare module 'acme-shims' { + const shim: string; + export { shim }; +} + +if (app.initializers.has('flarum-tags')) { + console.log('tags'); +} +`) + ).toBe( + await format(`declare module 'acme-shims' { + const shim: string; + export { shim }; +} + if ('flarum-tags' in flarum.extensions) { console.log('tags'); } From 4032a664ae777feb8b7cb9591945a92401062aa2 Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 17 Oct 2026 13:43:46 +0100 Subject: [PATCH 2/6] fix(upgrade): import from ext: wherever code names an extension's module, and keep its typings mapping --- .../frontend/export-registry/import-ext.ts | 35 ++--- test/steps/upgrade/import-ext.test.ts | 134 ++++++++++++++++++ 2 files changed, 149 insertions(+), 20 deletions(-) create mode 100644 test/steps/upgrade/import-ext.test.ts diff --git a/src/steps/upgrade/twopointoh/frontend/export-registry/import-ext.ts b/src/steps/upgrade/twopointoh/frontend/export-registry/import-ext.ts index 56dade4..aa792cc 100644 --- a/src/steps/upgrade/twopointoh/frontend/export-registry/import-ext.ts +++ b/src/steps/upgrade/twopointoh/frontend/export-registry/import-ext.ts @@ -93,21 +93,14 @@ export default class ImportExt extends BaseUpgradeStep { updateImportsFromFlarumExtensions(): Replacement { return (_file, code) => { // replace `import module from 'flarum/ext/module-path';` with `import module from 'ext:flarum/extension/module-path';` - // extension must not be one of [forum, admin, common] - const regex = new RegExp( - /import\s+(\w+)\s+from\s+["'](flarum\/(?!\b(?:common|admin|forum|utils|helpers|components|models)\b).*\/[\w/]+)["']/gim - ); - - // First collect all the imports that are from extensions - const imports = code.match(regex); - - if (imports?.length) { - this.importingFromExtensions = [ - ...this.importingFromExtensions, - ...imports.map((imp) => { - return imp.match(/from ["'](flarum\/\w+).*\/([\w/]+)["']/)![1]; - }), - ]; + // wherever the code names a module: in imports and exports of any kind, import(), require(), + // and `declare module`, which augments the module. extension must not be one of [forum, admin, common]. + const regex = + /(\bfrom\s*|\bimport\s*\(\s*|\brequire\s*\(\s*|\bdeclare\s+module\s+|\bimport\s+)(["'])(flarum\/(?!\b(?:common|admin|forum|utils|helpers|components|models)\b)[\w-]+\/[^\s"']+)\2/gm; + + // First collect all the extensions imported from + for (const match of code.matchAll(regex)) { + this.importingFromExtensions.push(match[3].split('/').slice(0, 2).join('/')); } // add todos to update imports that use other extensions @@ -118,7 +111,7 @@ export default class ImportExt extends BaseUpgradeStep { return { updated: code - .replace(regex, `import $1 from 'ext:$2'`) + .replace(regex, '$1$2ext:$3$2') // imports from @flarum/core no longer supported, replace with flarum/ .replace(/import\s+(\w+)\s+from\s+["']@flarum\/core\/([\w/]+)["']/gim, `import $1 from 'flarum/$2'`), }; @@ -138,11 +131,13 @@ export default class ImportExt extends BaseUpgradeStep { const paths = advanced.compilerOptions.paths; uniqueExtensions.forEach((ext) => { - if (paths[ext]) { - delete paths[ext]; - } + // Where the 1.x mapping found the typings, which isn't always vendor. + const typings = paths[`${ext}/*`]; + + delete paths[ext]; + delete paths[`${ext}/*`]; - paths[`ext:${ext}`] = [`../vendor/${ext}/js/dist-typings/*`]; + paths[`ext:${ext}/*`] ||= typings ?? [`../vendor/${ext}/js/dist-typings/*`]; }); return { diff --git a/test/steps/upgrade/import-ext.test.ts b/test/steps/upgrade/import-ext.test.ts new file mode 100644 index 0000000..373b5e3 --- /dev/null +++ b/test/steps/upgrade/import-ext.test.ts @@ -0,0 +1,134 @@ +import ImportExt from '../../../src/steps/upgrade/twopointoh/frontend/export-registry/import-ext'; + +/** Each file in turn through one step, as the upgrade does: the tsconfig comes last. */ +async function upgrade(files: Record): Promise> { + const step = new ImportExt({} as any, '/extension', 1, 1, false); + const upgraded: Record = {}; + + for (const [file, code] of Object.entries(files)) { + // eslint-disable-next-line no-await-in-loop + const result = await step.applyReplacements(file, code, step.advancedContent(file, code)); + + upgraded[file] = result.updated as string; + } + + return upgraded; +} + +const tsconfig = (paths: Record) => + JSON.stringify( + { extends: 'flarum-tsconfig', compilerOptions: { paths: { 'flarum/*': ['../vendor/flarum/core/js/dist-typings/*'], ...paths } } }, + null, + 2 + ); + +describe('the import from extensions step, on code', () => { + it("imports a bundled extension's modules from ext:, however the code imports them", async () => { + const { 'js/src/forum/index.ts': code } = await upgrade({ + 'js/src/forum/index.ts': `import Tag from 'flarum/tags/common/models/Tag'; +import type TagListState from 'flarum/tags/common/states/TagListState'; +import { slug } from "flarum/tags/common/utils/slug"; +import * as tagUtils from 'flarum/tags/common/utils'; +import Flag, { flags } from 'flarum/flags/forum/models/Flag'; +import 'flarum/sticky/forum/addStickyBadge'; +export { default as TagsPage } from 'flarum/tags/forum/components/TagsPage'; +export * from 'flarum/tags/common/helpers'; + +const loadModal = () => import('flarum/tags/forum/components/TagDiscussionModal'); +const Tags = require('flarum/tags/common/models/Tag'); +type TagModel = typeof import('flarum/tags/common/models/Tag'); + +declare module 'flarum/tags/common/models/Tag' { + export default interface Tag { + language(): string; + } +} +`, + }); + + // The step formats what it changes, so the quotes are Prettier's. + expect(code).toBe(`import Tag from 'ext:flarum/tags/common/models/Tag'; +import type TagListState from 'ext:flarum/tags/common/states/TagListState'; +import { slug } from 'ext:flarum/tags/common/utils/slug'; +import * as tagUtils from 'ext:flarum/tags/common/utils'; +import Flag, { flags } from 'ext:flarum/flags/forum/models/Flag'; +import 'ext:flarum/sticky/forum/addStickyBadge'; +export { default as TagsPage } from 'ext:flarum/tags/forum/components/TagsPage'; +export * from 'ext:flarum/tags/common/helpers'; + +const loadModal = () => import('ext:flarum/tags/forum/components/TagDiscussionModal'); +const Tags = require('ext:flarum/tags/common/models/Tag'); +type TagModel = typeof import('ext:flarum/tags/common/models/Tag'); + +declare module 'ext:flarum/tags/common/models/Tag' { + export default interface Tag { + language(): string; + } +} +`); + }); + + it("leaves core's modules, and the 1.x shortcuts to them, as they are", async () => { + const source = `import app from 'flarum/forum/app'; +import Model from 'flarum/common/Model'; +import Button from 'flarum/components/Button'; +import icon from 'flarum/helpers/icon'; +import ItemList from 'flarum/utils/ItemList'; +import Discussion from 'flarum/models/Discussion'; +import AdminPage from 'flarum/admin/components/AdminPage'; +`; + + expect((await upgrade({ 'js/src/forum/index.ts': source }))['js/src/forum/index.ts']).toBe(source); + }); + + it('changes nothing the second time', async () => { + const once = (await upgrade({ 'js/src/forum/index.ts': "import Tag from 'flarum/tags/common/models/Tag';\n" }))['js/src/forum/index.ts']; + + expect((await upgrade({ 'js/src/forum/index.ts': once }))['js/src/forum/index.ts']).toBe(once); + }); +}); + +describe('the import from extensions step, on the tsconfig', () => { + const paths = async (code: string, config: string) => + JSON.parse((await upgrade({ 'js/src/forum/index.ts': code, 'js/tsconfig.json': config }))['js/tsconfig.json']).compilerOptions.paths; + + it("maps each extension's ext: modules to its typings, in place of the 1.x mapping", async () => { + expect( + await paths( + "import type Tag from 'flarum/tags/common/models/Tag';\n", + tsconfig({ 'flarum/tags/*': ['../vendor/flarum/tags/js/dist-typings/*'] }) + ) + ).toStrictEqual({ + 'flarum/*': ['../vendor/flarum/core/js/dist-typings/*'], + 'ext:flarum/tags/*': ['../vendor/flarum/tags/js/dist-typings/*'], + }); + }); + + it('keeps where the 1.x mapping found the typings', async () => { + expect( + await paths("import Tag from 'flarum/tags/common/models/Tag';\n", tsconfig({ 'flarum/tags/*': ['../../packages/tags/js/dist-typings/*'] })) + ).toStrictEqual({ + 'flarum/*': ['../vendor/flarum/core/js/dist-typings/*'], + 'ext:flarum/tags/*': ['../../packages/tags/js/dist-typings/*'], + }); + }); + + it('maps an extension the 1.x tsconfig did not, from vendor', async () => { + expect(await paths("import Flag from 'flarum/flags/forum/models/Flag';\n", tsconfig({}))).toStrictEqual({ + 'flarum/*': ['../vendor/flarum/core/js/dist-typings/*'], + 'ext:flarum/flags/*': ['../vendor/flarum/flags/js/dist-typings/*'], + }); + }); + + it('keeps an ext: mapping the tsconfig already has', async () => { + expect( + await paths( + "import Tag from 'flarum/tags/common/models/Tag';\n", + tsconfig({ 'flarum/tags/*': ['../vendor/flarum/tags/js/dist-typings/*'], 'ext:flarum/tags/*': ['./typings/tags/*'] }) + ) + ).toStrictEqual({ + 'flarum/*': ['../vendor/flarum/core/js/dist-typings/*'], + 'ext:flarum/tags/*': ['./typings/tags/*'], + }); + }); +}); From db0ee03f1056bdf21fd0d5620a4cc27c8c04102f Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 17 Oct 2026 13:43:46 +0100 Subject: [PATCH 3/6] fix(upgrade): give a method assigned to a lazy module's prototype with override() --- .../code-splitting/usage-of-lazy-modules.ts | 112 +++++++++- test/steps/upgrade/lazy-modules.test.ts | 207 ++++++++++++++++++ 2 files changed, 312 insertions(+), 7 deletions(-) diff --git a/src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules.ts b/src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules.ts index fc23bd5..c2f9e95 100644 --- a/src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules.ts +++ b/src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules.ts @@ -202,6 +202,7 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { updateComposerLoad(): Replacement { return (_file, code, advanced) => { const ast = advanced as t.File; + let addsOverride = false; this.forEachLazyModule(code, ({ module, importPath }) => { // before = app.composer.load(${module}, { ... }, ...); @@ -209,7 +210,11 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { const lines: (t.ExpressionStatement | t.MemberExpression)[] = []; - const composers = this.composers; + // A composer's prototype lines move into the then() of the file's own + // load of it, which has the module. Without one, they are given like + // any other lazy module's. + const movesToLoad = this.composers.includes(module) && loadsComposer(ast, module); + const overrideName = importedName(ast, 'flarum/common/extend', 'override'); // pick up {module}.prototype... lines to add to the .then(() => { lines }) at the end of the call expression. traverse(ast, { @@ -240,19 +245,20 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { if (object.name !== module) return; - if (composers.includes(module)) { + if (movesToLoad) { lines.push(node); path.remove(); } else if ( (t.isFunctionExpression(expression.right) || t.isArrowFunctionExpression(expression.right)) && - (t.isIdentifier(left.property) || t.isStringLiteral(left.property)) + ((t.isIdentifier(left.property) && !left.computed) || t.isStringLiteral(left.property)) ) { - // SettingsPage.prototype['draftsItems'] = ...; - // change to extend('flarum/forum/components/SettingsPage', 'draftItems', ...); + // SettingsPage.prototype['draftsItems'] = function () {...}; + // change to override('flarum/forum/components/SettingsPage', 'draftsItems', function (original) {...}); const methodName = t.isIdentifier(left.property) ? left.property.name : left.property.value; - const extend = t.callExpression(t.identifier('extend'), [t.stringLiteral(importPath), t.stringLiteral(methodName), expression.right]); - path.replaceWith(t.expressionStatement(extend)); + path.replaceWith(overrideMethod(overrideName ?? 'override', importPath, methodName, expression.right)); + + if (!overrideName) addsOverride = true; } }, }); @@ -367,6 +373,7 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { return { updated: ast, + imports: addsOverride ? [{ import: { name: 'override', path: 'flarum/common/extend', defaultImport: false } }] : [], }; }; } @@ -535,3 +542,94 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { } } } + +/** Whether the file loads the composer itself, with app.composer.load(Composer, ...). */ +function loadsComposer(ast: t.File, module: string): boolean { + let loads = false; + + traverse(ast, { + CallExpression(path) { + const callee = path.node.callee; + const firstArg = path.node.arguments[0]; + + if ( + t.isMemberExpression(callee) && + t.isIdentifier(callee.property, { name: 'load' }) && + t.isMemberExpression(callee.object) && + t.isIdentifier(callee.object.property, { name: 'composer' }) && + t.isIdentifier(callee.object.object, { name: 'app' }) && + t.isIdentifier(firstArg, { name: module }) + ) { + loads = true; + path.stop(); + } + }, + }); + + return loads; +} + +/** The name a file imports one of a module's exports by, if it does. */ +function importedName(ast: t.File, source: string, name: string): string | null { + for (const node of ast.program.body) { + if (!t.isImportDeclaration(node) || node.source.value !== source) continue; + + for (const specifier of node.specifiers) { + if (t.isImportSpecifier(specifier) && t.isIdentifier(specifier.imported, { name })) { + return specifier.local.name; + } + } + } + + return null; +} + +/** A name for a new binding in the function, that hides none it uses. */ +function unusedName(node: t.Node, name: string): string { + const used = new Set(); + + t.traverseFast(node, (child) => { + if (t.isIdentifier(child)) used.add(child.name); + }); + + while (used.has(name)) name = '_' + name; + + return name; +} + +/** + * Module.prototype.method = fn; given with override() by the module's path, + * which replaces the method as the assignment did once the module loads. The + * original method comes first, after a TypeScript this parameter: a function + * that reads its arguments with `arguments` is called with them as they were. + */ +function overrideMethod( + override: string, + importPath: string, + methodName: string, + method: t.FunctionExpression | t.ArrowFunctionExpression +): t.ExpressionStatement { + const original = t.identifier(unusedName(method, 'original')); + let replacement: t.FunctionExpression | t.ArrowFunctionExpression; + + let readsArguments = false; + + t.traverseFast(method, (child) => { + if (t.isIdentifier(child, { name: 'arguments' })) readsArguments = true; + }); + + if (t.isFunctionExpression(method) && readsArguments) { + const args = t.identifier(unusedName(method, 'args')); + const call = t.callExpression(t.memberExpression(method, t.identifier('apply')), [t.thisExpression(), args]); + + replacement = t.functionExpression(null, [original, t.restElement(args)], t.blockStatement([t.returnStatement(call)])); + } else { + const first = method.params[0]; + const at = t.isIdentifier(first, { name: 'this' }) ? 1 : 0; + + replacement = t.cloneNode(method); + replacement.params.splice(at, 0, original); + } + + return t.expressionStatement(t.callExpression(t.identifier(override), [t.stringLiteral(importPath), t.stringLiteral(methodName), replacement])); +} diff --git a/test/steps/upgrade/lazy-modules.test.ts b/test/steps/upgrade/lazy-modules.test.ts index 02c8333..3851a9e 100644 --- a/test/steps/upgrade/lazy-modules.test.ts +++ b/test/steps/upgrade/lazy-modules.test.ts @@ -1,4 +1,5 @@ import UsageOfLazyModules from '../../../src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules'; +import { formatCode } from '../../../src/utils/ast'; async function upgrade(code: string, file = 'js/src/forum/index.js'): Promise { const step = new UsageOfLazyModules({} as any, '/extension', 1, 1, false); @@ -7,6 +8,8 @@ async function upgrade(code: string, file = 'js/src/forum/index.js'): Promise formatCode(code); + const todo = (module: string, path: string) => `// @TODO: ${module} is lazy loaded in 2.0, so this import is undefined until its chunk has loaded. Load it with \`await import('${path}')\` before using it. https://docs.flarum.org/2.x/extend/code-splitting`; @@ -70,3 +73,207 @@ export const isSignUp = (modal) => modal instanceof SignUpModal; expect((await upgrade(once)).split('@TODO').length).toBe(2); }); }); + +describe('the lazy modules step, on methods given to a prototype', () => { + // In 2.0 the module is undefined until its chunk has loaded, so the method is + // given with override() by the module's path: it then replaces the method as + // the assignment did, with the same arguments, return value and this. + + it("gives one to a lazy module's prototype with override, by the module's path", async () => { + // As fof/drafts' 1.x forum has it. + expect( + await upgrade(`import ItemList from 'flarum/common/utils/ItemList'; +import SettingsPage from 'flarum/forum/components/SettingsPage'; + +SettingsPage.prototype.draftsItems = function () { + const items = new ItemList(); + + return items; +}; +`) + ).toBe( + await format(`import { override } from 'flarum/common/extend'; +import ItemList from 'flarum/common/utils/ItemList'; + +override('flarum/forum/components/SettingsPage', 'draftsItems', function (original) { + const items = new ItemList(); + + return items; +}); +`) + ); + }); + + it("gives one to a composer's prototype the same way, when the file does not load that composer", async () => { + expect( + await upgrade(`import { extend } from 'flarum/common/extend'; +import DiscussionComposer from 'flarum/forum/components/DiscussionComposer'; + +export default function () { + DiscussionComposer.prototype.chooseLanguage = function (hide, callback) { + this.hide = hide; + + if (callback) callback(); + }; +} +`) + ).toBe( + await format(`import { extend, override } from 'flarum/common/extend'; + +export default function () { + override('flarum/forum/components/DiscussionComposer', 'chooseLanguage', function (original, hide, callback) { + this.hide = hide; + + if (callback) callback(); + }); +} +`) + ); + }); + + it('keeps a this parameter first, where TypeScript needs it', async () => { + // As fof/discussion-language's 1.x composer code has it. + const code = await upgrade( + `import DiscussionComposer from 'flarum/forum/components/DiscussionComposer'; + +DiscussionComposer.prototype.chooseLanguage = function (this: DiscussionComposer, hide: boolean, callback: (() => void) | null) { + if (callback) callback(); +}; +`, + 'js/src/forum/addLanguageComposer.tsx' + ); + + expect(code).toMatch( + /override\(\s*'flarum\/forum\/components\/DiscussionComposer',\s*'chooseLanguage',\s*function \(this: DiscussionComposer, original, hide: boolean, callback: \(\(\) => void\) \| null\) {/ + ); + }); + + it('gives one that is an arrow function, or named by a string, the same way', async () => { + expect( + await upgrade(`import { override } from 'flarum/common/extend'; +import SettingsPage from 'flarum/forum/components/SettingsPage'; + +SettingsPage.prototype.label = (name) => name.toUpperCase(); +SettingsPage.prototype['tagline'] = function () { + return 'Settings'; +}; +`) + ).toBe( + await format(`import { override } from 'flarum/common/extend'; + +override('flarum/forum/components/SettingsPage', 'label', (original, name) => name.toUpperCase()); +override('flarum/forum/components/SettingsPage', 'tagline', function (original) { + return 'Settings'; +}); +`) + ); + }); + + it('names the original method so it hides nothing the method uses', async () => { + const code = await upgrade(`import SettingsPage from 'flarum/forum/components/SettingsPage'; + +const original = 'Settings'; + +SettingsPage.prototype.tagline = function () { + return original; +}; +`); + + expect(code).toContain("override('flarum/forum/components/SettingsPage', 'tagline', function (_original) {\n return original;\n});"); + }); + + it('keeps the arguments a method reads with arguments where they were', async () => { + const code = await upgrade(`import SettingsPage from 'flarum/forum/components/SettingsPage'; + +SettingsPage.prototype.log = function () { + console.log(arguments); +}; +`); + + expect(code).toContain(`override('flarum/forum/components/SettingsPage', 'log', function (original, ...args) { + return function () { + console.log(arguments); + }.apply(this, args); +});`); + }); + + it('calls override by the name the file imports it with', async () => { + expect( + await upgrade(`import { override as replace } from 'flarum/common/extend'; +import SettingsPage from 'flarum/forum/components/SettingsPage'; + +SettingsPage.prototype.tagline = function () { + return 'Settings'; +}; +`) + ).toBe( + await format(`import { override as replace } from 'flarum/common/extend'; + +replace('flarum/forum/components/SettingsPage', 'tagline', function (original) { + return 'Settings'; +}); +`) + ); + }); + + it('leaves one whose name it cannot tell, for the author', async () => { + const code = await upgrade(`import SettingsPage from 'flarum/forum/components/SettingsPage'; + +const method = 'tagline'; + +SettingsPage.prototype[method] = function () { + return 'Settings'; +}; +`); + + expect(code).toContain('SettingsPage.prototype[method] = function () {'); + expect(code).not.toContain('override('); + }); + + it("moves a composer's into the then() of the file's own load of it", async () => { + // Where the file loads the composer itself, it has the module there. + expect( + await upgrade(`import app from 'flarum/forum/app'; +import DiscussionComposer from 'flarum/forum/components/DiscussionComposer'; + +DiscussionComposer.prototype.chooseLanguage = function () { + return 'language'; +}; + +export function compose() { + app.composer.load(DiscussionComposer, { user: app.session.user }); + app.composer.show(); +} +`) + ).toBe( + await format(`import app from 'flarum/forum/app'; +${todo('DiscussionComposer', 'flarum/forum/components/DiscussionComposer')} +import DiscussionComposer from 'flarum/forum/components/DiscussionComposer'; + +export function compose() { + app.composer + .load(() => import('flarum/forum/components/DiscussionComposer'), { user: app.session.user }) + .then((DiscussionComposer) => { + // @TODO: Move all direct access to the module object here. Including subsequent calls to app.composer.show(), checkout https://docs.flarum.org/2.x/extend/code-splitting#async-composers + DiscussionComposer.prototype.chooseLanguage = function () { + return 'language'; + }; + + app.composer.show(); + }); +} +`) + ); + }); + + it('changes nothing the second time', async () => { + const once = await upgrade(`import SettingsPage from 'flarum/forum/components/SettingsPage'; + +SettingsPage.prototype.tagline = function () { + return 'Settings'; +}; +`); + + expect(await upgrade(once)).toBe(once); + }); +}); From 711d2ad1f6c71c26460b2c02ea7717e51679a34a Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 17 Oct 2026 13:43:46 +0100 Subject: [PATCH 4/6] fix(upgrade): add an extender to the array extend.php returns, not to a Conditional's --- .../src/NodeVisitors/AddExtender.php | 50 ++++++++++- test/provider/php-provider.test.ts | 83 +++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/php-subsystem/src/NodeVisitors/AddExtender.php b/php-subsystem/src/NodeVisitors/AddExtender.php index 2213ea0..f8bd8d8 100644 --- a/php-subsystem/src/NodeVisitors/AddExtender.php +++ b/php-subsystem/src/NodeVisitors/AddExtender.php @@ -28,9 +28,57 @@ public function __construct($params, NameContext $nameContext) $this->nameContext = $nameContext; } + /** + * The return statements whose array of extenders gets the new one. + * + * @var array + */ + protected $targets = []; + + /** + * Picks the arrays of extenders the file returns: those returned outside any + * function, or failing that, from the fewest nested functions. An array a + * Conditional's closure returns is left alone, as it holds the extenders for + * when another extension is enabled. + */ + public function beforeTraverse(array $nodes) + { + $returns = []; + + $find = function (array $nodes, int $depth) use (&$find, &$returns) { + foreach ($nodes as $node) { + if (! $node instanceof Node) { + continue; + } + + if ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) { + $returns[] = [$node, $depth]; + } + + $nested = $node instanceof Node\FunctionLike ? $depth + 1 : $depth; + + foreach ($node->getSubNodeNames() as $name) { + $find(is_array($node->$name) ? $node->$name : [$node->$name], $nested); + } + } + }; + + $find($nodes, 0); + + $outermost = min(array_column($returns, 1) ?: [0]); + + foreach ($returns as [$return, $depth]) { + if ($depth === $outermost) { + $this->targets[spl_object_id($return)] = true; + } + } + + return null; + } + public function leaveNode(Node $node) { - if ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_) { + if ($node instanceof Node\Stmt\Return_ && $node->expr instanceof Node\Expr\Array_ && isset($this->targets[spl_object_id($node)])) { if (! isset($this->params['extender'])) { $params = json_encode($this->params); throw new \Exception("Invalid Extender Schema. Provided params: $params"); diff --git a/test/provider/php-provider.test.ts b/test/provider/php-provider.test.ts index a39af32..4d08cb3 100644 --- a/test/provider/php-provider.test.ts +++ b/test/provider/php-provider.test.ts @@ -103,3 +103,86 @@ describe('PhpProvider Works', function () { }).toThrow(); }); }); + +describe('adding an extender to extend.php', function () { + const provider = new PhpSubsystemProvider(path.resolve(__dirname, '../../php-subsystem/index.php')); + + const classConst = (value: string) => ({ type: ExpressionType.CLASS_CONST, value, auxiliaryValue: 'class' }); + + const listener: ExtenderDef = { + // Fully qualified with a leading backslash, as the steps that add extenders give it. + extender: { className: '\\Flarum\\Extend\\Event' }, + methodCalls: [{ methodName: 'listen', args: [classConst('Flarum\\Post\\Event\\Saving'), classConst('Acme\\Thing\\Listener\\SavePost')] }], + }; + + const optional = ` (new Extend\\Conditional()) + ->whenExtensionEnabled('flarum-tags', function () { + return [ + (new Extend\\Event()) + ->listen(Saving::class, SaveTags::class), + ]; + }),`; + + test.each([ + ['namespaced', '\nnamespace Acme\\Thing;\n'], + ['without a namespace', ''], + ])('adds it to the array the file returns, not to one a Conditional returns (%s)', function (_shape, namespace) { + const code = provider.withExtender( + `listen(Saving::class, SaveTags::class), +]; +`, + listener + ); + + expect(code.split('new Extend\\Event()').length).toBe(2); + expect(code).toMatch(/->listen\(Saving::class, SaveTags::class\)\s*->listen\(\\Flarum\\Post\\Event\\Saving::class, Listener\\SavePost::class\)/); + }); + + test('adds it to the array a wrapping function returns, when that is the outermost one', function () { + const code = provider.withExtender( + ` Date: Sat, 17 Oct 2026 13:43:47 +0100 Subject: [PATCH 5/6] fix(upgrade): register a filter once, type only constrain()'s query, and translate a gambit's hint --- .../src/Upgrade/TwoPointOh/Search.php | 110 +++++++- .../upgrade/twopointoh/backend/search.ts | 50 +++- test/steps/upgrade/search.test.ts | 255 ++++++++++++++++++ 3 files changed, 389 insertions(+), 26 deletions(-) create mode 100644 test/steps/upgrade/search.test.ts diff --git a/php-subsystem/src/Upgrade/TwoPointOh/Search.php b/php-subsystem/src/Upgrade/TwoPointOh/Search.php index 8225194..53fd31e 100644 --- a/php-subsystem/src/Upgrade/TwoPointOh/Search.php +++ b/php-subsystem/src/Upgrade/TwoPointOh/Search.php @@ -230,6 +230,39 @@ public function __construct($returnExtendersNode) $this->returnExtendersNode = $returnExtendersNode; } + /** The class a registration names, by the name it has once it is a filter. */ + private function registeredClass(Node\Expr $expr): ?string + { + if ($expr instanceof Node\Expr\ClassConstFetch && $expr->class instanceof Node\Name) { + $name = ($expr->class->getAttribute('resolvedName') ?? $expr->class)->toString(); + } elseif ($expr instanceof Node\Scalar\String_) { + $name = $expr->value; + } else { + return null; + } + + $name = str_replace(['FilterGambit', 'GambitFilter'], 'Filter', $name); + + return ltrim(str_replace('Gambit', 'Filter', $name), '\\'); + } + + /** + * 1.x registered a class that was both a gambit and a filter + * twice, with each extender: 2.0 needs it once. + */ + private function register(string $searcher, string $kind, Node\Expr $expr): void + { + $class = $this->registeredClass($expr); + + foreach ($this->data['searchers'][$searcher][$kind] ?? [] as $registered) { + if ($class !== null && $this->registeredClass($registered) === $class) { + return; + } + } + + $this->data['searchers'][$searcher][$kind][] = $expr; + } + private function findNew(Node $node): ?Node\Expr\New_ { if ($node instanceof Node\Expr\New_) { @@ -292,9 +325,9 @@ public function leaveNode(Node $node) } if ($method === 'addFilter') { - $this->data['searchers'][$searcher]['filters'][] = $arg->value; + $this->register($searcher, 'filters', $arg->value); } elseif ($method === 'addFilterMutator') { - $this->data['searchers'][$searcher]['mutators'][] = $arg->value; + $this->register($searcher, 'mutators', $arg->value); } } elseif ($class === 'Flarum\Extend\SimpleFlarumSearch') { $searcher = NodeUtil::className($new->args[0]->value->class); @@ -318,7 +351,7 @@ public function leaveNode(Node $node) } if ($method === 'addGambit') { - $this->data['searchers'][$searcher]['filters'][] = $arg->value; + $this->register($searcher, 'filters', $arg->value); } elseif ($method === 'setFullTextGambit') { $this->data['searchers'][$searcher]['fulltext'] = isset($resolvedName) ? $resolvedName->name @@ -381,13 +414,48 @@ public function __construct($visitor) $collectorData = $collector->data; } elseif (isset($visitor)) { $collectorData = [ - 'extendData' => $visitor->data, + 'extendData' => $this->reportable($visitor->data), ]; } return new ReplacementResult($code, $collectorData); } + /** + * The searchers the extenders registered, with each filter and mutator by + * the name it is written with, which is what the step reads. Not their + * syntax trees: a fully qualified name's refers to itself, which can't be + * encoded as JSON. + */ + protected function reportable(array $data): array + { + $searchers = []; + + foreach ($data['searchers'] ?? [] as $searcher => $registered) { + foreach (['filters', 'mutators'] as $kind) { + if (! isset($registered[$kind])) { + continue; + } + + $classes = []; + + foreach ($registered[$kind] as $expr) { + if ($expr instanceof Node\Scalar\String_) { + $classes[] = ['class' => ['name' => $expr->value]]; + } elseif ($expr instanceof Node\Expr\ClassConstFetch && $expr->class instanceof Node\Name) { + $classes[] = ['class' => ['name' => $expr->class->name]]; + } + } + + $registered[$kind] = $classes; + } + + $searchers[$searcher] = $registered; + } + + return ['searchers' => $searchers]; + } + function replaceNamespaces(string $file, string $code, array $ast, array $data): ?ReplacementResult { $traverser = $this->traverser(); @@ -439,16 +507,32 @@ function changeSignatures(string $file, string $code, array $ast, array $data): ], 'return' => ['void'], ], - 'constrain' => [ - 'params' => [ - 'query' => ['\\Illuminate\\Database\\Eloquent\\Builder'], - 'actor' => ['\\Flarum\\User\\User'], - 'negate' => ['bool'], - ], - 'return' => ['void'], - ], ], - ])); + ], function (Node $node) { + // A filter's constrain() helper is its own, so its parameters are + // left as they are, but for the query it is given: 2.0 makes that + // an Eloquent builder. + if (! $node instanceof Node\Stmt\ClassMethod || strtolower($node->name->name) !== 'constrain') { + return; + } + + foreach ($node->params as $param) { + $nullable = $param->type instanceof Node\NullableType; + $type = $nullable ? $param->type->type : $param->type; + + if (! $type instanceof Node\Name || ltrim(NodeUtil::className($type), '\\') !== 'Illuminate\\Database\\Query\\Builder') { + continue; + } + + $eloquent = NodeUtil::makeType(['\\Illuminate\\Database\\Eloquent\\Builder']); + + if ($nullable) { + $param->type->type = $eloquent; + } else { + $param->type = $eloquent; + } + } + })); return new ReplacementResult($traverser->traverse($ast)); } diff --git a/src/steps/upgrade/twopointoh/backend/search.ts b/src/steps/upgrade/twopointoh/backend/search.ts index e5d275c..097467b 100644 --- a/src/steps/upgrade/twopointoh/backend/search.ts +++ b/src/steps/upgrade/twopointoh/backend/search.ts @@ -7,6 +7,8 @@ import { GenerateGambitStub } from '../../../stubs/frontend/gambit'; import { GenerateSearchGambitExtender } from '../../../js/search-gambit'; import { LocaleStep } from '../../../locale/base'; import { pluralKebabCaseModel } from '../../../../utils/model-name'; +import fs from 'fs'; +import path from 'path'; type Gambit = { name: string; @@ -15,6 +17,9 @@ type Gambit = { filterFile: string; }; +/** A gambit with an is: pattern, such as is:private, rather than a key and value. */ +const isBoolean = (gambit: Gambit) => gambit.pattern.includes('is:'); + export default class Search extends BaseUpgradeStep { type = 'Search & Filter API changes'; beforeHook = true; @@ -136,12 +141,6 @@ export default class Search extends BaseUpgradeStep { const result = await this.command.runSteps( new StepManager().silentGroup((steps) => { this.gambits.forEach((gambit, index) => { - let gambitKey = gambit.pattern.replace('is:', ''); - - if (gambitKey.includes(':')) { - gambitKey = gambitKey.split(':')[0]; - } - const relatedSearcher = Object.keys(this.data.collectedData.extendData.searchers).find((searcher) => { const filters = this.data.collectedData.extendData.searchers[searcher].filters || []; return filters.some((filter: any) => { @@ -155,11 +154,11 @@ export default class Search extends BaseUpgradeStep { type = pluralKebabCaseModel(relatedSearcher.split('\\').pop()!.replace('Searcher', '')); } - steps + const gambitSteps = steps .namedStep(`gambit-${index}`, new GenerateGambitStub(this.command.STUB_PATH, genExtScaffolder()), {}, [], { frontend: 'common', className: gambit.name, - type: gambit.pattern.includes('is:') ? 'boolean' : 'key-value', + type: isBoolean(gambit) ? 'boolean' : 'key-value', filterKey: gambit.filterKey, }) .step( @@ -176,11 +175,11 @@ export default class Search extends BaseUpgradeStep { frontend: 'common', modelType: type, } - ) - .step(new LocaleStep(genExtScaffolder()), {}, [], { - key: `lib.gambits.${gambit.filterKey}.key`, - value: gambitKey, - }); + ); + + this.gambitTranslations(gambit).forEach((translation) => { + gambitSteps.step(new LocaleStep(genExtScaffolder()), {}, [], translation); + }); delete this.gambits[index]; }); @@ -209,6 +208,31 @@ export default class Search extends BaseUpgradeStep { ]; } + /** + * The translations a gambit's stub uses: its key, and a key-value gambit's + * hint, which the search shows beside it. Only the author can say what the + * hint should be, so it is left for them to write. + */ + gambitTranslations(gambit: Gambit): { key: string; value: string }[] { + let gambitKey = gambit.pattern.replace('is:', ''); + + if (gambitKey.includes(':')) { + gambitKey = gambitKey.split(':')[0]; + } + + const translations = [{ key: `lib.gambits.${gambit.filterKey}.key`, value: gambitKey }]; + + if (!isBoolean(gambit)) { + const hint = `lib.gambits.${gambit.filterKey}.hint`; + const locale = fs.existsSync(path.join(this.root, 'resources/locale/en.yml')) ? 'resources/locale/en.yml' : 'locale/en.yml'; + + translations.push({ key: hint, value: gambitKey }); + this.leaveTodo(locale, `describe what the ${gambitKey} gambit takes in ${hint}, which the search shows beside it.`); + } + + return translations; + } + targets(): string[] { return ['src/**/*', 'extend.php']; } diff --git a/test/steps/upgrade/search.test.ts b/test/steps/upgrade/search.test.ts new file mode 100644 index 0000000..106654a --- /dev/null +++ b/test/steps/upgrade/search.test.ts @@ -0,0 +1,255 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { PhpSubsystemProvider } from '../../../src/providers/php-provider'; +import Search from '../../../src/steps/upgrade/twopointoh/backend/search'; + +const php = new PhpSubsystemProvider(path.resolve(__dirname, '../../../php-subsystem/index.php')); + +const upgrade = (file: string, code: string, data: Record = {}): string => php.run('upgrade.2-0.search', { file, code, data }).code; + +/** A 1.x filter, as the step has it once the gambit parts are gone. */ +const filter = (constrain: string, uses = 'use Illuminate\\Database\\Query\\Builder;\n') => `constrain($filterState->getQuery(), $negate, $codes); + } +${constrain} +} +`; + +describe('the search step, on a filter', () => { + it('gives filter() the 2.0 signature, and its body the new parameter names', () => { + const code = upgrade('src/Search/LanguageFilter.php', filter('')); + + expect(code).toContain(`use Flarum\\Search\\Filter\\FilterInterface; +use Flarum\\Search\\SearchState;`); + expect(code).toContain(` public function filter(SearchState $state, array|string $value, bool $negate): void + { + $codes = explode(',', trim($value, '"')); + + $this->constrain($state->getQuery(), $negate, $codes); + }`); + }); + + it("changes only the query builder's type in its own constrain() helper", () => { + // As fof/discussion-language's 1.x LanguageFilterGambit has it. The query a + // filter is given is an Eloquent builder in 2.0; the rest is the extension's. + const code = upgrade( + 'src/Search/LanguageFilter.php', + filter(` + protected function constrain(Builder $query, bool $negate, array $codes): void + { + $codes = array_filter($codes, fn ($c) => $c !== 'any'); + + if ($negate) { + $query->whereNotIn('discussions.language_id', $codes); + } else { + $query->whereIn('discussions.language_id', $codes); + } + }`) + ); + + expect(code).toContain(` + protected function constrain(\\Illuminate\\Database\\Eloquent\\Builder $query, bool $negate, array $codes): void + { + $codes = array_filter($codes, fn ($c) => $c !== 'any'); + + if ($negate) { + $query->whereNotIn('discussions.language_id', $codes); + } else { + $query->whereIn('discussions.language_id', $codes); + } + }`); + }); + + it("changes the query builder's type in a constrain() shaped as core's are", () => { + // As core 1.x's UnreadFilterGambit has it. + const code = upgrade( + 'src/Search/LanguageFilter.php', + filter( + ` + protected function constrain(Builder $query, User $actor, bool $negate): void + { + }`, + 'use Flarum\\User\\User;\nuse Illuminate\\Database\\Query\\Builder;\n' + ) + ); + + expect(code).toContain('protected function constrain(\\Illuminate\\Database\\Eloquent\\Builder $query, User $actor, bool $negate): void'); + }); + + it("changes the query builder's type wherever constrain() takes it, and whatever it returns", () => { + const code = upgrade( + 'src/Search/LanguageFilter.php', + filter(` + protected function constrain(array $codes, ?Builder $query = null) + { + return $query; + }`) + ); + + expect(code).toContain('protected function constrain(array $codes, ?\\Illuminate\\Database\\Eloquent\\Builder $query = null)\n'); + }); + + it('leaves a constrain() that already takes an Eloquent builder as it was', () => { + const constrain = ` + protected function constrain(Builder $query, bool $negate, array $codes): void + { + }`; + + const code = upgrade('src/Search/LanguageFilter.php', filter(constrain, 'use Illuminate\\Database\\Eloquent\\Builder;\n')); + + expect(code).toContain(constrain); + }); +}); + +describe('the search step, in extend.php', () => { + const extend = (registrations: string, uses = '') => ` { + // As fof/discussion-language's 1.x extend.php has it. + const code = upgrade( + 'extend.php', + extend(` (new Extend\\SimpleFlarumSearch(DiscussionSearcher::class)) + ->addGambit(Search\\LanguageFilterGambit::class), + + (new Extend\\Filter(DiscussionFilterer::class)) + ->addFilter(Search\\LanguageFilterGambit::class),`), + replacements + ); + + expect(code).toContain(`return [ + (new Extend\\SearchDriver(\\Flarum\\Search\\Database\\DatabaseSearchDriver::class)) + ->addFilter(DiscussionSearcher::class, Search\\LanguageFilter::class), +];`); + }); + + it('registers it once however each registration names the class', () => { + // Imported, fully qualified, or as a string: each is 1.x code in the wild. + const code = upgrade( + 'extend.php', + extend( + ` (new Extend\\SimpleFlarumSearch(DiscussionSearcher::class)) + ->addGambit(LanguageFilterGambit::class) + ->addGambit('Acme\\\\Thing\\\\Search\\\\LanguageFilterGambit'), + + (new Extend\\Filter(DiscussionFilterer::class)) + ->addFilter(\\Acme\\Thing\\Search\\LanguageFilterGambit::class),`, + 'use Acme\\Thing\\Search\\LanguageFilterGambit;\n' + ), + replacements + ); + + expect(code.match(/->addFilter\(/g)).toHaveLength(1); + }); + + it('reports the filters it found by the names they are registered with', () => { + // The step reads them to tell which searcher a gambit's filter belongs to. + const { extendData } = php.run('upgrade.2-0.search', { + file: 'extend.php', + code: extend(` (new Extend\\Filter(DiscussionFilterer::class)) + ->addFilter(\\Acme\\Thing\\Search\\LanguageFilterGambit::class) + ->addFilter('Acme\\\\Thing\\\\Search\\\\CountryFilterGambit') + ->addFilter(Search\\RegionFilterGambit::class),`), + data: replacements, + }); + + expect(extendData.searchers['Flarum\\Discussion\\Search\\DiscussionSearcher'].filters).toStrictEqual([ + { class: { name: 'Acme\\Thing\\Search\\LanguageFilter' } }, + { class: { name: 'Acme\\Thing\\Search\\CountryFilter' } }, + { class: { name: 'Search\\RegionFilter' } }, + ]); + }); + + it('keeps each of several filters and mutators, once each', () => { + const code = upgrade( + 'extend.php', + extend(` (new Extend\\SimpleFlarumSearch(DiscussionSearcher::class)) + ->addGambit(Search\\LanguageFilterGambit::class) + ->addGambit(Search\\CountryFilterGambit::class), + + (new Extend\\Filter(DiscussionFilterer::class)) + ->addFilter(Search\\LanguageFilterGambit::class) + ->addFilter(Search\\CountryFilterGambit::class) + ->addFilterMutator(Search\\HideOthers::class) + ->addFilterMutator(Search\\HideOthers::class),`), + replacements + ); + + expect(code).toContain(` (new Extend\\SearchDriver(\\Flarum\\Search\\Database\\DatabaseSearchDriver::class)) + ->addFilter(DiscussionSearcher::class, Search\\LanguageFilter::class) + ->addFilter(DiscussionSearcher::class, Search\\CountryFilter::class) + ->addMutator(DiscussionSearcher::class, Search\\HideOthers::class),`); + }); +}); + +describe('the search step, on the gambits it moves to the frontend', () => { + const step = () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'search-gambits-')); + + fs.mkdirSync(path.join(root, 'resources/locale'), { recursive: true }); + fs.writeFileSync(path.join(root, 'resources/locale/en.yml'), 'acme-thing: {}\n'); + + return new Search({} as any, root, 1, 1, false); + }; + + it("translates a key-value gambit's key and the hint its stub shows, and leaves the hint to write", () => { + // As fof/discussion-language's 1.x LanguageFilterGambit has it. + const search = step(); + + expect( + search.gambitTranslations({ + name: 'LanguageGambit', + pattern: 'language:(.+)', + filterKey: 'language', + filterFile: 'src/Search/LanguageFilter.php', + }) + ).toStrictEqual([ + { key: 'lib.gambits.language.key', value: 'language' }, + { key: 'lib.gambits.language.hint', value: 'language' }, + ]); + + expect((search as any).followUps).toEqual([ + { file: 'resources/locale/en.yml', line: null, text: expect.stringContaining('lib.gambits.language.hint') }, + ]); + }); + + it("translates a boolean gambit's key, as its stub has no hint", () => { + const search = step(); + + expect( + search.gambitTranslations({ name: 'PrivateGambit', pattern: 'is:private', filterKey: 'private', filterFile: 'src/Search/PrivateFilter.php' }) + ).toStrictEqual([{ key: 'lib.gambits.private.key', value: 'private' }]); + + expect((search as any).followUps).toEqual([]); + }); +}); From 515b15acdb330c5bb5d1cf298a3da5f98544f1f3 Mon Sep 17 00:00:00 2001 From: IanM Date: Sat, 17 Oct 2026 13:43:47 +0100 Subject: [PATCH 6/6] fix: require flarum-webpack-config 3.0.7, which exports a default export as its value --- boilerplate/skeleton/extension/js/package.json | 2 +- src/steps/upgrade/twopointoh/dependencies.ts | 5 +++-- test/steps/upgrade/dependencies.test.ts | 18 ++++++++++++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/boilerplate/skeleton/extension/js/package.json b/boilerplate/skeleton/extension/js/package.json index 35b2a32..bf34aeb 100644 --- a/boilerplate/skeleton/extension/js/package.json +++ b/boilerplate/skeleton/extension/js/package.json @@ -10,7 +10,7 @@ "@flarum/jest-config": "^2.0.0", "@flarum/prettier-config": "^1.0.0", "flarum-tsconfig": "^2.0.0", - "flarum-webpack-config": "^3.0.6", + "flarum-webpack-config": "^3.0.7", "prettier": "^2.5.1", "typescript": "^4.9.5", "typescript-coverage-report": "^0.6.1", diff --git a/src/steps/upgrade/twopointoh/dependencies.ts b/src/steps/upgrade/twopointoh/dependencies.ts index 7069a47..fa8d99d 100644 --- a/src/steps/upgrade/twopointoh/dependencies.ts +++ b/src/steps/upgrade/twopointoh/dependencies.ts @@ -275,9 +275,10 @@ export default class Dependencies extends BaseUpgradeStep { // The versions a new extension is scaffolded with. 3.0.6 is the first // flarum-webpack-config that loads an extension's code-split chunks from - // its own build. + // its own build, and 3.0.7 the first that exports a default export as its + // value, not under a name taken from its code. const map: Record = { - 'flarum-webpack-config': '^3.0.6', + 'flarum-webpack-config': '^3.0.7', 'flarum-tsconfig': '^2.0.0', '@flarum/jest-config': '^2.0.0', webpack: '^5.104.1', diff --git a/test/steps/upgrade/dependencies.test.ts b/test/steps/upgrade/dependencies.test.ts index 0313a2d..00e4637 100644 --- a/test/steps/upgrade/dependencies.test.ts +++ b/test/steps/upgrade/dependencies.test.ts @@ -120,14 +120,20 @@ describe('the dependencies step', () => { it('updates the build tooling in both dependencies and devDependencies', async () => { const pkg = await upgrade('js/package.json', onePackageJson); - expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.6'); + expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.7'); expect(pkg.devDependencies['flarum-tsconfig']).toBe('^2.0.0'); }); - it('requires flarum-webpack-config 3.0.6, which loads code-split chunks from the right build', async () => { + it('requires flarum-webpack-config 3.0.7, which loads code-split chunks from the right build and exports a default export as its value', async () => { const pkg = await upgrade('js/package.json', { devDependencies: { 'flarum-webpack-config': '^3.0.0' } }); - expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.6'); + expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.7'); + }); + + it('moves an extension already on flarum-webpack-config 3.0.6 to 3.0.7', async () => { + const pkg = await upgrade('js/package.json', { devDependencies: { 'flarum-webpack-config': '^3.0.6' } }); + + expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.7'); }); it('requires the webpack Flarum builds with', async () => { @@ -187,7 +193,7 @@ describe('the dependencies step', () => { }); expect(pkg.dependencies).toEqual({}); - expect(pkg.devDependencies).toEqual({ 'flarum-webpack-config': '^3.0.6', prettier: '^3.0.3' }); + expect(pkg.devDependencies).toEqual({ 'flarum-webpack-config': '^3.0.7', prettier: '^3.0.3' }); }); it('still drops a devDependency the extension also lists as a dependency, when it is not tooling', async () => { @@ -285,10 +291,10 @@ describe('the dependencies step', () => { }); describe('a new extension', () => { - it('is scaffolded with flarum-webpack-config 3.0.6 and the webpack Flarum builds with', () => { + it('is scaffolded with flarum-webpack-config 3.0.7 and the webpack Flarum builds with', () => { const scaffolded = scaffoldedPackageJson().devDependencies; - expect(scaffolded['flarum-webpack-config']).toBe('^3.0.6'); + expect(scaffolded['flarum-webpack-config']).toBe('^3.0.7'); expect(scaffolded.webpack).toBe('^5.104.1'); }); });