diff --git a/src/steps/upgrade/twopointoh/base.ts b/src/steps/upgrade/twopointoh/base.ts index 7e78854..49edd0f 100644 --- a/src/steps/upgrade/twopointoh/base.ts +++ b/src/steps/upgrade/twopointoh/base.ts @@ -304,6 +304,12 @@ export abstract class BaseUpgradeStep implements Step { // eslint-disable-next-line no-await-in-loop code = await this.updateCode(file, code, result.updated); + // Later replacements read the parsed code: parse it again after a text + // change, or one that returns a syntax tree would undo this change. + if (typeof result.updated === 'string') { + advanced = this.advancedContent(file, code); + } + if (result.imports?.length || (result.updated && typeof result.updated !== 'string' && 'program' in result.updated)) { try { advanced = parseCode(code); diff --git a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts index 152e367..5e34a25 100644 --- a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts +++ b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts @@ -7,6 +7,7 @@ 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 { extensionId } from '../../../../utils/composer'; const TODO = '@TODO: register this with Extend.Admin in admin/extend instead'; const DOCS = 'https://docs.flarum.org/2.x/extend/admin'; @@ -462,10 +463,7 @@ export default class AdminExtender extends BaseUpgradeStep { /** The extension's ID, as core makes it from the package name. */ protected extensionId(): string | null { try { - const name: string = JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8')).name; - const [vendor, pkg] = name.includes('/') ? name.split('/', 2) : [name, name]; - - return `${vendor}-${pkg.replace(/flarum-ext-/g, '').replace(/flarum-/g, '')}`; + return extensionId(JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8')).name); } catch { return null; } diff --git a/src/steps/upgrade/twopointoh/frontend/misc.ts b/src/steps/upgrade/twopointoh/frontend/misc.ts index ec40a45..8e32228 100644 --- a/src/steps/upgrade/twopointoh/frontend/misc.ts +++ b/src/steps/upgrade/twopointoh/frontend/misc.ts @@ -2,7 +2,48 @@ import { BaseUpgradeStep, GitCommit, ImportChange, Replacement } from '../base'; import chalk from 'chalk'; 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 { extensionId } from '../../../../utils/composer'; + +/** The bundled extensions' IDs, which are their initializers' names in 2.0. */ +const BUNDLED = new Set([ + 'flarum-akismet', + 'flarum-approval', + 'flarum-audit', + 'flarum-bbcode', + 'flarum-deck', + 'flarum-embed', + 'flarum-emoji', + 'flarum-extension-manager', + 'flarum-flags', + 'flarum-gdpr', + 'flarum-lang-english', + 'flarum-likes', + 'flarum-lock', + 'flarum-markdown', + 'flarum-mentions', + 'flarum-messages', + 'flarum-nicknames', + 'flarum-pusher', + 'flarum-realtime', + 'flarum-statistics', + 'flarum-sticky', + 'flarum-subscriptions', + 'flarum-suspend', + 'flarum-tags', +]); + +/** The 1.x names of the bundled initializers 2.0 renamed (flarum/framework#4003). */ +const RENAMED: Record = { + lock: 'flarum-lock', + subscriptions: 'flarum-subscriptions', + 'flarum/nicknames': 'flarum-nicknames', +}; + +const CHECK_TODO = "@TODO: check whether the extension is enabled with '' in flarum.extensions instead"; export default class MiscFrontendChanges extends BaseUpgradeStep { type = 'Miscellaneous frontend changes.'; @@ -387,19 +428,124 @@ export default class MiscFrontendChanges extends BaseUpgradeStep { }; } + /** + * `app.initializers.has(name)` becomes `'id' in flarum.extensions`, where the + * upgrade knows the ID for certain: a bundled extension's, or one the + * extension's composer.json names. An initializer is not always named after + * its extension, so a check it can't be sure of gets a @TODO instead. + */ private updateInitializers(): Replacement { return (_file, code) => { - if (!code.includes('this.currentTag')) return null; + if (!/initializers\s*\.\s*has\s*\(/.test(code)) return null; + + const edits: Array<{ start: number; end: number; text: string }> = []; + const flagged = new Set(); + + traverse(parse(code) as t.File, { + CallExpression: (p) => { + const id = this.checkedExtension(p.node); + + if (id === undefined) return; + + if (id) { + edits.push({ start: p.node.start!, end: p.node.end!, text: `('${id}' in flarum.extensions)` }); + + return; + } + + // Above the statement, where a comment can't land in JSX or a string. + const statement = p.getStatementParent(); + + if (!statement || statement.node.leadingComments?.some((comment) => comment.value.includes(CHECK_TODO))) return; + + const lineStart = code.lastIndexOf('\n', statement.node.start! - 1) + 1; + + if (flagged.has(lineStart)) return; + + flagged.add(lineStart); + + const indent = /^[\t ]*/.exec(code.slice(lineStart))![0]; + const docs = 'https://docs.flarum.org/2.x/extend/extending-extensions'; + + edits.push({ + start: lineStart, + end: lineStart, + text: `${indent}// ${CHECK_TODO} (${docs}). An initializer is not always named after its extension, and some were renamed in 2.0.\n`, + }); + }, + }); + + if (edits.length === 0) return null; return { - updated: code - .replace(/initializers\.has\(["']lock["']\)/g, "initializers.has('flarum-lock')") - .replace(/initializers\.has\(["']subscriptions["']\)/g, "initializers.has('flarum-subscriptions')") - .replace(/initializers\.has\(["']flarum\/nicknames["']\)/g, "initializers.add('flarum-nicknames')"), + updated: edits + .sort((a, b) => b.start - a.start) + .reduce((result, edit) => result.slice(0, edit.start) + edit.text + result.slice(edit.end), code), }; }; } + /** + * For `app.initializers.has('name')`: the ID of the extension it checks for, + * or null when the upgrade can't be sure of it. Undefined for anything else. + */ + private checkedExtension(call: t.CallExpression): string | null | undefined { + const callee = call.callee; + + if (!t.isMemberExpression(callee) || callee.computed || !t.isIdentifier(callee.property, { name: 'has' })) return undefined; + + const initializers = callee.object; + + if (!t.isMemberExpression(initializers) || initializers.computed || !t.isIdentifier(initializers.property, { name: 'initializers' })) { + return undefined; + } + + const owner = initializers.object; + + if (!t.isIdentifier(owner, { name: 'app' }) && !(t.isMemberExpression(owner) && t.isIdentifier(owner.property, { name: 'app' }))) { + return undefined; + } + + const [name] = call.arguments; + + if (call.arguments.length !== 1 || !t.isStringLiteral(name)) return undefined; + + return RENAMED[name.value] ?? (BUNDLED.has(name.value) ? name.value : this.namedExtensions().get(name.value) ?? null); + } + + private named: Map | null = null; + + /** + * The extensions the composer.json names, by package name and by ID. + */ + private namedExtensions(): Map { + if (this.named) return this.named; + + this.named = new Map(); + + let composer: Record = {}; + + try { + composer = JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8')); + } catch { + return this.named; + } + + const packages = [ + ...Object.keys(composer.require ?? {}), + ...Object.keys(composer['require-dev'] ?? {}), + ...Object.keys(composer.suggest ?? {}), + ...(composer.extra?.['flarum-extension']?.['optional-dependencies'] ?? []), + ].filter((name: unknown): name is string => typeof name === 'string' && name.includes('/')); + + for (const name of packages) { + this.named.set(name, extensionId(name)); + this.named.set(extensionId(name), extensionId(name)); + } + + return this.named; + } + private updateUploadImageButton(): Replacement { return (file, code) => { if (!code.includes('UploadImageButton')) return null; diff --git a/src/utils/composer.ts b/src/utils/composer.ts index 4eea539..2b922ed 100644 --- a/src/utils/composer.ts +++ b/src/utils/composer.ts @@ -70,3 +70,13 @@ export type ComposerJsonSchema = { export function getComposerJson(fs: Store, paths: Paths): ComposerJsonSchema { return create(fs).readJSON(paths.package('composer.json')) as ComposerJsonSchema; } + +/** + * An extension's ID, from its package name, as core makes it: `vendor-package`, + * without a `flarum-ext-` or `flarum-` in the package's part. + */ +export function extensionId(packageName: string): string { + const [vendor, pkg] = packageName.includes('/') ? packageName.split('/', 2) : [packageName, packageName]; + + return `${vendor}-${pkg.replace(/flarum-ext-/g, '').replace(/flarum-/g, '')}`; +} diff --git a/test/steps/upgrade/initializer-checks.test.ts b/test/steps/upgrade/initializer-checks.test.ts new file mode 100644 index 0000000..129c10b --- /dev/null +++ b/test/steps/upgrade/initializer-checks.test.ts @@ -0,0 +1,167 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import MiscFrontendChanges from '../../../src/steps/upgrade/twopointoh/frontend/misc'; +import { formatCode } from '../../../src/utils/ast'; + +/** + * In 1.x, code checked whether another extension was enabled with + * app.initializers.has(name). Initializer names are not always the extension's + * ID, and some of core's changed in 2.0, so the upgrade checks the extension's + * ID in flarum.extensions instead, where it knows the ID for certain. + */ + +function extension(composer: Record): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'initializer-checks-')); + + fs.writeFileSync(path.join(root, 'composer.json'), JSON.stringify({ name: 'acme/flarum-interstellar', ...composer })); + + return root; +} + +async function upgrade(code: string, composer: Record = {}, file = 'js/src/forum/index.ts'): Promise { + const step = new MiscFrontendChanges({} as any, extension(composer), 1, 1, false); + const result = await step.applyReplacements(file, code, step.advancedContent(file, code)); + + return result.updated as string; +} + +const format = (code: string) => formatCode(code); + +const todo = + "// @TODO: check whether the extension is enabled with '' in flarum.extensions instead (https://docs.flarum.org/2.x/extend/extending-extensions). An initializer is not always named after its extension, and some were renamed in 2.0."; + +describe('checking whether another extension is enabled', () => { + it("uses a bundled extension's ID", async () => { + // As fof/sitemap's 1.x forum has it. + expect( + await upgrade(`import app from 'flarum/forum/app'; + +if (app.initializers.has('flarum-tags')) { + console.log('tags'); +} +`) + ).toBe( + await format(`import app from 'flarum/forum/app'; + +if ('flarum-tags' in flarum.extensions) { + console.log('tags'); +} +`) + ); + }); + + it('uses the IDs of the bundled extensions whose initializers 2.0 renamed', async () => { + // fof/username-request's 1.x forum checks 'flarum/nicknames', which 2.0 + // renamed 'flarum-nicknames', so the check is false in 2.0. + expect( + await upgrade(`const nicknames = app.initializers.has('flarum/nicknames'); +const lock = app.initializers.has('lock'); +const subscriptions = app.initializers.has("subscriptions"); +`) + ).toBe( + await format(`const nicknames = 'flarum-nicknames' in flarum.extensions; +const lock = 'flarum-lock' in flarum.extensions; +const subscriptions = 'flarum-subscriptions' in flarum.extensions; +`) + ); + }); + + it('uses the ID of an extension the composer.json names, by its ID or its package name', async () => { + // As ianm/follow-users' and fof/drafts' 1.x forums have it. + const composer = { + require: { 'flarum/core': '^1.8', 'fof/follow-tags': '*' }, + 'require-dev': { 'clarkwinkelmann/flarum-ext-author-change': '*' }, + suggest: { 'fof/user-directory': 'Lists users' }, + extra: { 'flarum-extension': { 'optional-dependencies': ['fof/byobu'] } }, + }; + + expect( + await upgrade( + `const tags = app.initializers.has('fof/follow-tags'); +const directory = app.initializers.has('fof-user-directory'); +const byobu = app.initializers.has('fof-byobu'); +const author = app.initializers.has('clarkwinkelmann-author-change'); +`, + composer + ) + ).toBe( + await format(`const tags = 'fof-follow-tags' in flarum.extensions; +const directory = 'fof-user-directory' in flarum.extensions; +const byobu = 'fof-byobu' in flarum.extensions; +const author = 'clarkwinkelmann-author-change' in flarum.extensions; +`) + ); + }); + + it('keeps what the check meant, whatever it is part of', async () => { + expect( + await upgrade(`const shown = !app.initializers.has('flarum-tags') && app.initializers.has('flarum-sticky') ? 1 : 2; +`) + ).toBe( + await format(`const shown = !('flarum-tags' in flarum.extensions) && 'flarum-sticky' in flarum.extensions ? 1 : 2; +`) + ); + }); + + it('leaves a check on an extension whose ID it cannot know, with a @TODO above its statement', async () => { + // As datlechin/flarum-scroll-buttons' 1.x forum has it: neither extension + // is in its composer.json. The @TODO goes above the statement, not inside + // the JSX, where it would show. + const code = `export default function view() { + return ( +
+ {app.initializers.has('acpl/mobile-tab') && app.initializers.has('itnt-uitab') ? : null} +
+ ); +} +`; + + expect(await upgrade(code, {}, 'js/src/forum/view.tsx')).toBe( + await format(`export default function view() { + ${todo} + return ( +
+ {app.initializers.has('acpl/mobile-tab') && app.initializers.has('itnt-uitab') ? : null} +
+ ); +} +`) + ); + }); + + it("keeps its changes, and the misc step's other ones, when a later change rewrites the syntax tree", async () => { + // As fof/impersonate's 1.x ImpersonateModal has it: the modal becomes a + // FormModal by rewriting the syntax tree, after the text changes ran. + const code = await upgrade(`import Modal from 'flarum/common/components/Modal'; + +export default class ImpersonateModal extends Modal { + oninit(vnode) { + super.oninit(vnode); + + this.reasonEnabled = app.initializers.has('flarum-tags'); + this.tag = this.currentTag; + } + + content() { + return ; + } +} +`); + + expect(code).toContain('extends FormModal'); + expect(code).toContain("this.reasonEnabled = 'flarum-tags' in flarum.extensions;"); + expect(code).toContain('this.tag = app.currentTag;'); + }); + + it('changes nothing the second time', async () => { + const once = await upgrade(`if (app.initializers.has('flarum-tags') || app.initializers.has('acme-other')) { + console.log('tags'); +} +`); + + expect(once).toContain("'flarum-tags' in flarum.extensions"); + expect(once.match(/@TODO/g)).toHaveLength(1); + expect(await upgrade(once)).toBe(once); + }); +});