From 4c14f3fd73aaf16f2b4fad81799bf1e75ca8f5c1 Mon Sep 17 00:00:00 2001 From: IanM Date: Fri, 16 Oct 2026 11:01:04 +0100 Subject: [PATCH] fix(upgrade): correct what a real 1.x extension's upgrade got wrong, and add --check --- README.md | 6 +- .../skeleton/extension/js/tsconfig.json | 3 + .../skeleton/extension/js/tsconfig.test.json | 1 + php-subsystem/src/NodeUtil.php | 28 +- .../src/Upgrade/TwoPointOh/JsonApi.php | 22 +- php-subsystem/src/Upgrade/TwoPointOh/Misc.php | 30 +- src/base-command.ts | 10 +- src/commands/upgrade/2.0.ts | 155 +++++++++- .../upgrade/twopointoh/backend/email-views.ts | 58 ++-- .../upgrade/twopointoh/backend/json-api.ts | 19 ++ src/steps/upgrade/twopointoh/backend/misc.ts | 33 ++- src/steps/upgrade/twopointoh/base.ts | 53 +++- src/steps/upgrade/twopointoh/dependencies.ts | 268 +++++++++++++++++- .../twopointoh/frontend/admin-extender.ts | 54 +++- .../code-splitting/usage-of-lazy-modules.ts | 40 ++- src/steps/upgrade/twopointoh/frontend/misc.ts | 20 ++ src/steps/upgrade/twopointoh/infra.ts | 83 +++++- src/steps/upgrade/twopointoh/todos.ts | 3 +- test/steps/upgrade/admin-extender.test.ts | 79 ++++-- test/steps/upgrade/dependencies.test.ts | 168 ++++++++++- test/steps/upgrade/infra.test.ts | 91 +++++- test/steps/upgrade/json-api.test.ts | 69 +++++ test/steps/upgrade/json-layout.test.ts | 47 +++ test/steps/upgrade/json-report.test.ts | 156 +++++++++- test/steps/upgrade/lazy-modules.test.ts | 72 +++++ test/steps/upgrade/misc-backend.test.ts | 85 ++++++ test/steps/upgrade/phpunit.test.ts | 19 ++ test/steps/upgrade/typescript.test.ts | 87 +++++- 28 files changed, 1590 insertions(+), 169 deletions(-) create mode 100644 test/steps/upgrade/json-api.test.ts create mode 100644 test/steps/upgrade/json-layout.test.ts create mode 100644 test/steps/upgrade/lazy-modules.test.ts create mode 100644 test/steps/upgrade/misc-backend.test.ts diff --git a/README.md b/README.md index e24067c8..e2327e6f 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ The CLI has different types of commands for different tasks: **Code Updates**: These commands help update extensions for newer versions of Flarum. - `flarum-cli update js-imports [PATH]`: Adds admin/forum/common namespaces to all JS imports from flarum core. -- `flarum-cli upgrade 2.0 [PATH] [-n|--no-interaction] [--json] [--step ] [--force-steps]`: Upgrades a 1.x extension to Flarum 2.0, one step at a time, committing each step. Steps already committed are skipped, so running it again resumes where it stopped. +- `flarum-cli upgrade 2.0 [PATH] [-n|--no-interaction] [--json] [--check] [--step ] [--force-steps]`: Upgrades a 1.x extension to Flarum 2.0, one step at a time, committing each step. Steps already committed are skipped, so running it again resumes where it stopped. To drive the upgrade from another program, such as an AI agent, pass `--json`. It asks nothing, and prints a single JSON object when it finishes. It exits non-zero when the upgrade didn't complete: @@ -161,7 +161,9 @@ To drive the upgrade from another program, such as an AI agent, pass `--json`. I } ``` -Each step is `committed`, `no-changes`, `skipped` (committed by an earlier run), `failed` or `not-run`. A step that fails on some files lists them and makes no changes, so fix those files, commit, and run the command again. The `todos` are the `@TODO` comments the step's commit added, for what the upgrade could not change itself. +Each step is `committed`, `no-changes`, `skipped` (committed by an earlier run), `failed` or `not-run`. A step that fails on some files lists them and makes no changes, so fix those files, commit, and run the command again. The `todos` are what the upgrade could not change itself: the `@TODO` comments the step's commit added, and work it can only point out, such as a file to add, which has a `line` of `null`. A `no-changes` step lists its `todos` too, when it has any. + +To see what the upgrade would change without changing anything, pass `--check`, such as for an extension already on Flarum 2.0. The steps run on the extension's last commit, in a temporary git worktree that is removed afterwards, and none is skipped. With `--json`, a step that would change something is `would-change`, with the `files` and `diff` of the change, and the report's `check` names the commit checked and whether changes not yet committed were left out. _And of course, you can always use the help command to see a list of all available commands with their descriptions:_ diff --git a/boilerplate/skeleton/extension/js/tsconfig.json b/boilerplate/skeleton/extension/js/tsconfig.json index 504f8003..8804296e 100644 --- a/boilerplate/skeleton/extension/js/tsconfig.json +++ b/boilerplate/skeleton/extension/js/tsconfig.json @@ -12,6 +12,9 @@ "@types/**/*" ], "compilerOptions": { + // Only load the type packages the code imports. Others, such as the + // @types/node webpack brings along, use syntax TypeScript 4.9 can't read. + "types": [], // This will output typings to `dist-typings` "declarationDir": "./dist-typings", "baseUrl": ".", diff --git a/boilerplate/skeleton/extension/js/tsconfig.test.json b/boilerplate/skeleton/extension/js/tsconfig.test.json index be7e3082..0cef5e12 100644 --- a/boilerplate/skeleton/extension/js/tsconfig.test.json +++ b/boilerplate/skeleton/extension/js/tsconfig.test.json @@ -3,6 +3,7 @@ "include": ["tests/**/*"], "files": ["../../../node_modules/@flarum/jest-config/shims.d.ts"], "compilerOptions": { + "types": ["jest"], "strict": false, "noImplicitReturns": false, "noImplicitAny": false diff --git a/php-subsystem/src/NodeUtil.php b/php-subsystem/src/NodeUtil.php index e14effbc..08be7227 100644 --- a/php-subsystem/src/NodeUtil.php +++ b/php-subsystem/src/NodeUtil.php @@ -55,18 +55,28 @@ public static function addUsesToNamespace(Node\Stmt\Namespace_ $node, array $use return true; })); - $lastUseStatement = null; + usort($uses, fn ($a, $b) => strcasecmp($a->uses[0]->name->toString(), $b->uses[0]->name->toString())); - foreach ($node->stmts as $index => $stmt) { - if ($stmt instanceof \PhpParser\Node\Stmt\Use_) { - $lastUseStatement = $index; - } elseif (! is_null($lastUseStatement)) { - break; + // Each goes where it sorts among the imports already there, as the + // Flarum StyleCI preset orders them, or after the last one. + foreach ($uses as $use) { + $lastUseStatement = null; + $position = null; + + foreach ($node->stmts as $index => $stmt) { + if ($stmt instanceof \PhpParser\Node\Stmt\Use_) { + $lastUseStatement = $index; + + if (is_null($position) && strcasecmp($stmt->uses[0]->name->toString(), $use->uses[0]->name->toString()) > 0) { + $position = $index; + } + } elseif (! is_null($lastUseStatement)) { + break; + } } - } - if (! is_null($lastUseStatement)) { - array_splice($node->stmts, $lastUseStatement + 1, 0, $uses); + // Without any imports yet, it opens the namespace. + array_splice($node->stmts, $position ?? (is_null($lastUseStatement) ? 0 : $lastUseStatement + 1), 0, [$use]); } } diff --git a/php-subsystem/src/Upgrade/TwoPointOh/JsonApi.php b/php-subsystem/src/Upgrade/TwoPointOh/JsonApi.php index 9a40f378..5e075792 100644 --- a/php-subsystem/src/Upgrade/TwoPointOh/JsonApi.php +++ b/php-subsystem/src/Upgrade/TwoPointOh/JsonApi.php @@ -135,9 +135,9 @@ function extenders(string $file, string $code, array $ast): ?ReplacementResult $traverser->addVisitor(new NodeVisitor\ParentConnectingVisitor()); + // The TODO says what to write instead. Imports are left to whoever + // writes it: added now, they would go unused until then. $traverser->addVisitor(new class () extends \PhpParser\NodeVisitorAbstract { - public $hasExtenders = false; - const LINK_TO_DOCS = 'https://docs.flarum.org/2.x/extend/api#extending-api-resources'; const COMMENT = '// @TODO: Replace with the new implementation '.self::LINK_TO_DOCS; @@ -157,28 +157,10 @@ public function enterNode(\PhpParser\Node $node) if ($parent) { $parent->setAttribute('comments', [new \PhpParser\Comment\Doc(self::COMMENT)]); } - - $this->hasExtenders = true; } } } } - - public function leaveNode(\PhpParser\Node $node) - { - if (! $this->hasExtenders) { - return null; - } - - if ($node instanceof \PhpParser\Node\Stmt\Namespace_) { - NodeUtil::addUsesToNamespace($node, [ - 'Flarum\Api\Context', - 'Flarum\Api\Endpoint', - 'Flarum\Api\Resource', - 'Flarum\Api\Schema', - ]); - } - } }); return new ReplacementResult($traverser->traverse($ast)); diff --git a/php-subsystem/src/Upgrade/TwoPointOh/Misc.php b/php-subsystem/src/Upgrade/TwoPointOh/Misc.php index 50f4a3a7..9f72362e 100644 --- a/php-subsystem/src/Upgrade/TwoPointOh/Misc.php +++ b/php-subsystem/src/Upgrade/TwoPointOh/Misc.php @@ -2,6 +2,7 @@ namespace Flarum\CliPhpSubsystem\Upgrade\TwoPointOh; +use Flarum\CliPhpSubsystem\NodeUtil; use Flarum\CliPhpSubsystem\NodeVisitors\ChangeSignatures; use Flarum\CliPhpSubsystem\Upgrade\Replacement; use Flarum\CliPhpSubsystem\Upgrade\ReplacementResult; @@ -198,7 +199,11 @@ public function enterNode(\PhpParser\Node $node) if ($node instanceof \PhpParser\Node\Expr\MethodCall) { if ($class = $this->getClass($node)) { if ($class === 'Extend\Notification') { - if ($node->name->name === 'type') { + // 1.x's second argument is the serializer. 2.x's is the + // drivers enabled by default, an array, which stays. + $serializer = $node->args[1]->value ?? null; + + if ($node->name->name === 'type' && ($serializer instanceof \PhpParser\Node\Expr\ClassConstFetch || $serializer instanceof \PhpParser\Node\Scalar\String_)) { $args = $node->args; unset($args[1]); $node->args = array_values($args); @@ -371,14 +376,13 @@ function alertable(string $file, string $code, array $ast, array $data): ?Replac public function enterNode(Node $node) { - // Does this class implement the Flarum\Notification\Blueprint\BlueprintInterface ? + // Does this class implement the Flarum\Notification\Blueprint\BlueprintInterface, + // and not yet the Flarum\Notification\AlertableInterface ? if ($node instanceof Node\Stmt\Class_) { - foreach ($node->implements as $interface) { - if ($interface->getAttribute('resolvedName')->name === 'Flarum\Notification\Blueprint\BlueprintInterface') { - $this->valid = true; - break; - } - } + $implements = array_map(fn ($interface) => $interface->getAttribute('resolvedName')->name, $node->implements); + + $this->valid = in_array('Flarum\Notification\Blueprint\BlueprintInterface', $implements) + && ! in_array('Flarum\Notification\AlertableInterface', $implements); } } @@ -389,15 +393,7 @@ public function leaveNode(\PhpParser\Node $node) } if ($node instanceof Node\Stmt\Namespace_) { - // Add the Flarum\Notification\AlertableInterface import - $node->stmts = array_merge( - [ - new Node\Stmt\Use_([ - new Node\Stmt\UseUse(new Node\Name('Flarum\Notification\AlertableInterface')) - ]) - ], - $node->stmts - ); + NodeUtil::addUsesToNamespace($node, ['Flarum\Notification\AlertableInterface']); } if ($node instanceof \PhpParser\Node\Stmt\Class_) { diff --git a/src/base-command.ts b/src/base-command.ts index ef244f76..bb7754cb 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -60,7 +60,7 @@ export default abstract class BaseCommand extends Command { const { args } = await this.parse(this.constructor as any); this.args = args; - const path: string | undefined = args.path; + const path: string | undefined = this.stepsPath(args.path); const paths = new NodePaths({ requestedDir: path, @@ -72,6 +72,14 @@ export default abstract class BaseCommand extends Command { return steps.run(paths, this.genIO(), { php: phpProvider }, this.dry, args); } + /** + * Where the steps run, which is where the command was asked to unless a + * command runs them somewhere else. + */ + protected stepsPath(path: string | undefined): string | undefined { + return path; + } + async extRoot(path: string | undefined): Promise { let extRoot: string; diff --git a/src/commands/upgrade/2.0.ts b/src/commands/upgrade/2.0.ts index 63b2bb2c..1834a475 100644 --- a/src/commands/upgrade/2.0.ts +++ b/src/commands/upgrade/2.0.ts @@ -22,7 +22,30 @@ import { Flags } from '@oclif/core'; import EmailViews from '../../steps/upgrade/twopointoh/backend/email-views'; import InterventionImage from '../../steps/upgrade/twopointoh/backend/intervention-image'; import { BaseUpgradeStep } from '../../steps/upgrade/twopointoh/base'; -import { Message } from 'boilersmith/io'; +import { IO, Message } from 'boilersmith/io'; +import { execFileSync } from 'child_process'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import simpleGit from 'simple-git'; + +/** + * The temporary worktree a check runs the steps in, so the extension itself + * is never changed. + */ +type Checkout = { + /** The repository the extension is in. */ + repo: string; + worktree: string; + branch: string; + /** The extension's root inside the worktree. */ + root: string; + /** What was checked. */ + commit: string; + /** Whether the extension had changes not committed, which a check leaves out. */ + dirty: boolean; + cwd: string; +}; export default class TwoPointOh extends BaseCommand { static description = 'Upgrade an extension to Flarum 2.0'; @@ -44,6 +67,11 @@ export default class TwoPointOh extends BaseCommand { description: 'Force run steps even if already ran', required: false, }), + check: Flags.boolean({ + description: + 'Report what the upgrade would change, such as on an extension already on Flarum 2.0, without changing it: the steps run on the last commit, in a temporary git worktree that is removed afterwards. Implies --force-steps and --no-interaction.', + required: false, + }), }; static args = [...BaseCommand.args]; @@ -57,6 +85,87 @@ export default class TwoPointOh extends BaseCommand { */ protected scheduled: BaseUpgradeStep[] = []; + protected checkout: Checkout | null = null; + + async run(): Promise { + try { + await super.run(); + } finally { + await this.removeCheckout(); + } + } + + protected async additionalPreRunChecks(extRoot: string): Promise { + if (this.flags.check) { + this.checkout = await this.createCheckout(extRoot); + } + } + + protected stepsPath(requested: string | undefined): string | undefined { + return this.checkout?.root ?? requested; + } + + protected genIO(): IO { + // A check asks nothing: there is nothing to confirm. + if (this.flags?.check) this.flags['no-interaction'] = true; + + return super.genIO(); + } + + protected async createCheckout(extRoot: string): Promise { + const root = fs.realpathSync(extRoot); + const git = simpleGit(root); + + let repo: string; + + try { + repo = (await git.revparse(['--show-toplevel'])).trim(); + } catch { + this.error('--check runs the upgrade on a copy of the extension in a git worktree, so the extension must be in a git repository.', { + code: 'FL_ERR', + }); + } + + const commit = (await git.revparse(['HEAD'])).trim(); + const dirty = !(await git.status()).isClean(); + const worktree = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'flarum-upgrade-check-')); + const branch = `flarum-upgrade-check/${path.basename(worktree)}`; + + await simpleGit(repo).raw(['worktree', 'add', '--quiet', '-b', branch, worktree, commit]); + + return { repo, worktree, branch, root: path.join(worktree, path.relative(repo, root)), commit, dirty, cwd: process.cwd() }; + } + + /** Remove the worktree and its branch, saying so if it can't. */ + protected async removeCheckout(): Promise { + const checkout = this.checkout; + + if (!checkout) return; + + this.checkout = null; + + // The steps ran from inside the worktree. + process.chdir(checkout.cwd); + + try { + await simpleGit(checkout.repo).raw(['worktree', 'remove', '--force', checkout.worktree]); + await simpleGit(checkout.repo).raw(['branch', '-D', checkout.branch]); + } catch (error) { + process.stderr.write( + `The check could not remove its worktree ${checkout.worktree} and branch ${checkout.branch}: ${(error as Error).message}\n` + + `Remove them with: git worktree remove --force ${checkout.worktree} && git branch -D ${checkout.branch}\n` + ); + } + } + + /** What a step that committed in the worktree would change, from its commit. */ + protected wouldChange(sha: string): { files: string[]; diff: string } { + const show = (...args: string[]) => + execFileSync('git', ['show', '--format=', '--no-color', '--no-ext-diff', ...args, sha], { cwd: this.checkout!.worktree, encoding: 'utf8' }); + + return { files: show('--name-only').split('\n').filter(Boolean), diff: show() }; + } + protected steps(steps: StepManager, extRoot: string): StepManager { // Error.stackTraceLimit = Number.POSITIVE_INFINITY; @@ -98,7 +207,8 @@ export default class TwoPointOh extends BaseCommand { }); collection = collection.map((Step: new (...args: any) => any) => { - const step = new Step(this, extRoot, stepCount, total, this.flags['force-steps']); + const step = new Step(this, this.checkout?.root ?? extRoot, stepCount, total, this.flags['force-steps'] || this.flags.check); + step.checking = Boolean(this.flags.check); stepCount++; return step; }); @@ -127,15 +237,32 @@ export default class TwoPointOh extends BaseCommand { protected jsonReport(succeeded: boolean, error: string | null, messages: Message[]): Record { return { ...super.jsonReport(succeeded, error, messages), + ...(this.checkout && { check: { commit: this.checkout.commit, uncommittedChangesLeftOut: this.checkout.dirty } }), steps: this.scheduled.map((step) => ({ step: step.number, title: step.type, - ...(step.outcome ?? { status: 'not-run' }), + ...this.reported(step), })), }; } + /** A step's outcome, as what it would change when checking. */ + protected reported(step: BaseUpgradeStep): Record { + const outcome = step.outcome ?? { status: 'not-run' }; + + if (!this.checkout || outcome.status !== 'committed') return outcome; + + return { status: 'would-change', ...this.wouldChange(outcome.commit.sha), notes: outcome.notes, todos: outcome.todos }; + } + protected welcomeMessage(): string { + if (this.flags.check) { + return ` + Checking what the Flarum 2.0 upgrade would change in your extension. The steps run on its last commit, + in a temporary git worktree that is removed afterwards: nothing in your extension changes. + `; + } + const beforeProceeding = chalk.bold.red('Before proceeding:'); const command = chalk.bold.bgYellow.black('fl update js-imports'); const makeSure = chalk.bold.dim(`- Make sure your extend.php file directly returns an array of extenders. @@ -154,6 +281,28 @@ export default class TwoPointOh extends BaseCommand { } protected goodbyeMessage(): string { + if (this.checkout) { + const changing = this.scheduled.filter((step) => step.outcome?.status === 'committed'); + const lines = changing.map((step) => { + const { files } = this.wouldChange((step.outcome as { commit: { sha: string } }).commit.sha); + + return ` - Step ${step.number}, ${step.type} ${chalk.dim(files.join(', '))}`; + }); + + return [ + changing.length > 0 + ? ` ${changing.length} of the steps would change something in ${this.checkout.commit.slice(0, 7)}:` + : ` None of the steps would change anything in ${this.checkout.commit.slice(0, 7)}.`, + ...lines, + this.checkout.dirty && ' Changes that were not committed were left out of the check.', + '', + ' Nothing was changed. Run with --json as well to see each diff, or without --check to make the changes.', + '', + ] + .filter((line) => line !== false) + .join('\n'); + } + const composer = chalk.bold.bgYellow('composer update'); const yarn = chalk.bold.bgYellow('yarn install'); diff --git a/src/steps/upgrade/twopointoh/backend/email-views.ts b/src/steps/upgrade/twopointoh/backend/email-views.ts index 2b85b7bf..d66ccc5b 100644 --- a/src/steps/upgrade/twopointoh/backend/email-views.ts +++ b/src/steps/upgrade/twopointoh/backend/email-views.ts @@ -1,6 +1,9 @@ import ejs from 'ejs'; import { BaseUpgradeStep, GitCommit, Replacement } from '../base'; +/** Where a view lives: `acme-thing::emails.launched` is in `.../emails/launched.blade.php`. */ +const viewFile = (view: string): string => '/' + view.split('::').pop()!.replace(/\./g, '/') + '.blade.php'; + export default class EmailViews extends BaseUpgradeStep { type = 'Email notifications now require both Plain Text and HTML views.'; @@ -34,44 +37,33 @@ export default class EmailViews extends BaseUpgradeStep { }; }, async (file, code) => { - if (Object.keys(this.collected).length === 0) return null; + const newFiles: { path: string; code: string }[] = []; - const fileName = file.split('/').pop(); - const newFiles: any[] = []; + for (const views of Object.values(this.collected)) { + // A notification that has both views already has nothing to convert, + // such as one already upgraded to 2.0. + if (!views || Boolean(views.html) === Boolean(views.text)) continue; - Object.keys(this.collected).forEach((classFile) => { - if (!this.collected[classFile]) { - return; - } + const view = views.html || views.text; - if (!this.collected[classFile].html && !this.collected[classFile].text) { - return; - } + // The view's own file, by its full path: on 2.x the html and plain + // views of a notification have the same file name. + if (!file.endsWith(viewFile(view))) continue; - // if there is a text view but no html view, use the same content for both. - // same for html view but no text view. - - ['text', 'html'].forEach((view) => { - const viewType = view as 'html' | 'text'; - const viewPath = this.collected[classFile][viewType] || this.collected[classFile][viewType === 'html' ? 'text' : 'html']; - const viewFileName = viewPath.split('::')[1].split('.').pop(); - - if (fileName === `${viewFileName}.blade.php`) { - const templateContent = this.fsEditor!.read(__dirname + this.templates[viewType]); - const viewTypePath = viewType === 'text' ? 'plain' : 'html'; - newFiles.push({ - path: file.split('/views/')[0] + '/views/email/' + viewTypePath + '/' + viewFileName + '.blade.php', - code: ejs.render(templateContent, { content: code.trimEnd() }), - }); - } - }); - }); + const viewFileName = view.split('::')[1].split('.').pop(); - return { - updated: code, - delete: newFiles.length > 0, - newFiles, - }; + for (const viewType of ['text', 'html'] as const) { + const templateContent = this.fsEditor!.read(__dirname + this.templates[viewType]); + const viewTypePath = viewType === 'text' ? 'plain' : 'html'; + + newFiles.push({ + path: file.split('/views/')[0] + '/views/email/' + viewTypePath + '/' + viewFileName + '.blade.php', + code: ejs.render(templateContent, { content: code.trimEnd() }), + }); + } + } + + return newFiles.length > 0 ? { updated: code, delete: true, newFiles } : null; }, ]; } diff --git a/src/steps/upgrade/twopointoh/backend/json-api.ts b/src/steps/upgrade/twopointoh/backend/json-api.ts index c84211ad..a4ca6535 100644 --- a/src/steps/upgrade/twopointoh/backend/json-api.ts +++ b/src/steps/upgrade/twopointoh/backend/json-api.ts @@ -45,6 +45,7 @@ export default class JsonApi extends BaseUpgradeStep { let collected: any = null; return [ + this.flagAvatarUrl(), async (file, code) => { const output = this.php!.run('upgrade.2-0.json-api', { file, code }); @@ -104,6 +105,24 @@ export default class JsonApi extends BaseUpgradeStep { return ['src/**/*', 'extend.php']; } + /** + * 2.0's user resource has avatarSrcset next to avatarUrl, and browsers + * prefer it, so code that only sets the URL still shows a user's uploaded + * avatar on most screens. + */ + flagAvatarUrl(): Replacement { + return (file, code) => { + if (/["']avatarUrl["']/.test(code)) { + this.leaveTodo( + file, + "It sets avatarUrl. 2.0's API also has avatarSrcset, which browsers prefer over the URL: set it too, or a user's uploaded avatar still shows." + ); + } + + return null; + }; + } + gitCommit(): GitCommit { return { message: 'chore(2.0): JSON:API changes', diff --git a/src/steps/upgrade/twopointoh/backend/misc.ts b/src/steps/upgrade/twopointoh/backend/misc.ts index 61e3ee70..780dabbc 100644 --- a/src/steps/upgrade/twopointoh/backend/misc.ts +++ b/src/steps/upgrade/twopointoh/backend/misc.ts @@ -1,12 +1,18 @@ import { BaseUpgradeStep, GitCommit, Replacement } from '../base'; import chalk from 'chalk'; +const MODIFYING_COLUMNS = 'https://laravel.com/docs/11.x/upgrade#modifying-columns'; + export default class MiscBackendChanges extends BaseUpgradeStep { type = 'Miscellaneous backend changes.'; replacements(file: string): Replacement[] { if (!file.endsWith('.php')) return []; + if (file.startsWith('migrations/')) { + return [this.flagColumnChanges()]; + } + return [ (file, code) => ({ updated: this.php!.run('upgrade.2-0.misc', { file, code }).code, @@ -15,7 +21,7 @@ export default class MiscBackendChanges extends BaseUpgradeStep { } targets(): string[] { - return ['extend.php', 'src/**/*']; + return ['extend.php', 'src/**/*', 'migrations/**/*.php']; } gitCommit(): GitCommit { @@ -25,17 +31,30 @@ export default class MiscBackendChanges extends BaseUpgradeStep { }; } + /** + * Since Laravel 11, changing a column drops every attribute the change + * leaves out, so a migration's ->change() must repeat the whole column + * definition. Only the author knows the column's full definition. + */ + flagColumnChanges(): Replacement { + return (file, code) => { + if (/->\s*change\s*\(\s*\)/.test(code)) { + this.leaveTodo( + file, + `It changes a column with ->change(), which now drops every attribute the change leaves out: repeat the column's whole definition. ${MODIFYING_COLUMNS}` + ); + } + + return null; + }; + } + pauseMessage(): string { const link = 'https://docs.flarum.org/2.x/extend/update-2_0#miscellaneous-1'; const readMore = chalk.dim(`Read more: ${link}`); - const requiresManualChangeLink = 'https://laravel.com/docs/11.x/upgrade#modifying-columns'; return `Various breaking backend changes have been made in 2.0. The tool has attempted to update your code accordingly. Please review the changes and ensure everything is correct. - ${readMore} - - Additionally, check every reference to a column definition change in your migrations, and make sure - they are using the entire column definition. For more details, see: - ${chalk.underline(requiresManualChangeLink)}`; + ${readMore}`; } } diff --git a/src/steps/upgrade/twopointoh/base.ts b/src/steps/upgrade/twopointoh/base.ts index eaa2ec55..50993891 100644 --- a/src/steps/upgrade/twopointoh/base.ts +++ b/src/steps/upgrade/twopointoh/base.ts @@ -31,6 +31,14 @@ export type Replacement = (file: string, code: string, advanced: AdvancedContent export type AdvancedContent = t.File | Record | null; +/** + * The indentation a JSON file's first indented line uses, or the given + * fallback when it has none (an empty object, say, or everything on one line). + */ +function jsonIndentation(code: string, fallback: number): string | number { + return code.match(/^[\t ]+(?=\S)/m)?.[0] ?? fallback; +} + export type ImportChange = { replacesPath?: string | null; import: ModuleImport; @@ -45,7 +53,8 @@ export type GitCommit = { * What a step did, for a program driving the upgrade. */ export type StepOutcome = - | { status: 'skipped' | 'no-changes' } + | { status: 'skipped' } + | { status: 'no-changes'; todos?: Todo[] } | { status: 'committed'; commit: { sha: string; message: string }; notes: string; todos: Todo[] } | { status: 'failed'; failures: Failure[] }; @@ -95,6 +104,27 @@ export abstract class BaseUpgradeStep implements Step { abstract gitCommit(): GitCommit; abstract pauseMessage(): string; + /** Whether the step runs for a check, in a worktree that is thrown away. */ + public checking = false; + + /** Work this step leaves that no @TODO comment in a file shows. */ + protected followUps: Todo[] = []; + + /** + * Leave work for the author that the step can't mark with a @TODO comment, + * such as a file that is missing, or a change it can only point out. + */ + protected leaveTodo(file: string, text: string): void { + if (!this.followUps.some((todo) => todo.file === file && todo.text === text)) { + this.followUps.push({ file, line: null, text }); + } + } + + /** The follow-ups, as lines to print under the step's notes. */ + protected followUpLines(): string[] { + return this.followUps.map((todo) => `${chalk.bgYellow.black(' TO DO ')} ${todo.file}: ${todo.text}`); + } + protected fsEditor: null | Editor = null; /** @@ -293,7 +323,7 @@ export abstract class BaseUpgradeStep implements Step { const notes = this.pauseMessage(); - await this.pauseAndConfirm(notes, io); + await this.pauseAndConfirm([notes, ...this.followUpLines()].join('\n '), io); const commit = this.gitCommit(); const message = commit.message + '\n\n' + commit.description; @@ -305,15 +335,21 @@ export abstract class BaseUpgradeStep implements Step { status: 'committed', commit: { sha, message: commit.message }, notes: stripAnsi(notes).trim(), - todos: await todosAddedBy(dir, sha), + todos: [...(await todosAddedBy(dir, sha)), ...this.followUps], }; - this.command.log(overwrite() + ' => ' + chalk.bgCyan.bold(' COMMIT ') + ' ' + chalk.dim(commit.message)); + const label = this.checking ? chalk.bgYellow.black.bold(' WOULD CHANGE ') : chalk.bgCyan.bold(' COMMIT '); + this.command.log(overwrite() + ' => ' + label + ' ' + chalk.dim(commit.message)); this.command.log(''); } else { - this.outcome = { status: 'no-changes' }; + this.outcome = this.followUps.length > 0 ? { status: 'no-changes', todos: this.followUps } : { status: 'no-changes' }; this.command.log(overwrite() + ' => ' + chalk.bgGreen.bold(' NO CHANGES ')); + + for (const line of this.followUpLines()) { + this.command.log(' ' + line); + } + this.command.log(''); } @@ -449,9 +485,12 @@ export abstract class BaseUpgradeStep implements Step { } if (lang === 'json') { - const space = file === 'composer.json' ? 4 : 2; + // Write the file back the way it was laid out, so the diff only shows + // what the step changed. + const space = jsonIndentation(code, file === 'composer.json' ? 4 : 2); + const json = JSON.stringify(updated, null, space); - return JSON.stringify(updated, null, space); + return code.includes('\r\n') ? json.replace(/\n/g, '\r\n') + (code.endsWith('\r\n') ? '\r\n' : '') : json + (code.endsWith('\n') ? '\n' : ''); } if (updated && 'program' in updated) { diff --git a/src/steps/upgrade/twopointoh/dependencies.ts b/src/steps/upgrade/twopointoh/dependencies.ts index e255ca5d..7069a474 100644 --- a/src/steps/upgrade/twopointoh/dependencies.ts +++ b/src/steps/upgrade/twopointoh/dependencies.ts @@ -1,5 +1,7 @@ import { AdvancedContent, BaseUpgradeStep, GitCommit, Replacement } from './base'; import chalk from 'chalk'; +import fs from 'fs'; +import path from 'path'; import { compilerOptions, libs } from './typescript-4-9'; // What `fl init` gave 1.x extensions. `sed -i` with no suffix is GNU sed's @@ -17,6 +19,62 @@ const NEWER_SYNTAX: [RegExp, string][] = [ [/^\s*(?:await\s+)?using\s+[$A-Z_a-z][\w$]*\s*[:=]/, '`using` declarations (TypeScript 5.2)'], ]; +// Packages an extension only needs to build, check and test its frontend. +const BUILD_TOOLING = new Set([ + '@flarum/jest-config', + '@flarum/prettier-config', + 'flarum-tsconfig', + 'flarum-webpack-config', + 'prettier', + 'typescript', + 'typescript-coverage-report', + 'webpack', + 'webpack-cli', +]); + +// The scripts that build and check an extension's typings, as the reusable +// frontend workflow runs them. +const TYPINGS_SCRIPTS = ['clean-typings', 'build-typings', 'post-build-typings', 'check-typings', 'check-typings-coverage']; + +// What those scripts run. +const TYPINGS_TOOLING = ['typescript', 'typescript-coverage-report']; + +/** The package.json `fl init` gives a new extension, for the given package manager. */ +function scaffoldedPackageJson(packageManager: 'yarn' | 'npm'): Record { + const skeleton = fs.readFileSync(path.resolve(__dirname, '../../../../boilerplate/skeleton/extension/js/package.json'), 'utf8'); + + // EJS placeholders only appear inside strings, so it parses as JSON. + return JSON.parse(skeleton.replace(/<%= params\.jsPackageManager %>/g, packageManager)); +} + +const TYPES_COMMENT = [ + 'Only load the type packages the code imports. Others, such as the', + "@types/node webpack brings along, use syntax TypeScript 4.9 can't read.", +]; + +/** + * The tsconfig.json with an option first in its compilerOptions, and the + * comment lines above it, indented as the options there are. Null when it has + * no compilerOptions to put it in. + */ +function insertCompilerOption(code: string, comment: string[] | null, option: string): string | null { + const open = code.match(/"compilerOptions"\s*:\s*{/); + + if (!open) return null; + + const at = open.index! + open[0].length; + const eol = code.includes('\r\n') ? '\r\n' : '\n'; + const next = code.slice(at).match(/\r?\n([\t ]*)\S/); + const indent = next?.[1] ?? ' '; + const lines = [...(comment ?? []).map((line) => `// ${line}`), `${option},`]; + + return code.slice(0, at) + lines.map((line) => eol + indent + line).join('') + code.slice(at); +} + +/** Dependencies in the alphabetical order package managers keep them in. */ +const sortedByName = (dependencies: Record) => + Object.fromEntries(Object.entries(dependencies).sort(([a], [b]) => a.localeCompare(b))); + const isTypeScript = (file: string) => /\.tsx?$/.test(file); /** Whether a version constraint stays within TypeScript 4: `4.9.5`, `^4.5.4`, `~4.7`, `4.x`. */ @@ -26,6 +84,29 @@ const isFourX = (constraint: string) => { return version === '4' || version.startsWith('4.'); }; +/** + * The lowest version a simple constraint allows, as [major, minor, patch]: + * `^5.1.4`, `~4.7`, `>=8.3`, `5.x`. Null for anything else, such as `*`, a + * range with `||` or a prerelease. + */ +const lowestVersion = (constraint: string): number[] | null => { + const match = constraint.trim().match(/^(?:\^|~|>=|=)?\s*v?(\d+)(?:\.(\d+|x|\*))?(?:\.(\d+|x|\*))?$/); + + return match ? match.slice(1).map((part) => (part && /^\d+$/.test(part) ? Number(part) : 0)) : null; +}; + +/** Whether a constraint asks for a newer version than another does. */ +const asksForNewer = (constraint: string, than: string): boolean => { + const asked = lowestVersion(constraint); + const other = lowestVersion(than); + + if (!asked || !other) return false; + + const differs = asked.findIndex((part, i) => part !== other[i]); + + return differs !== -1 && asked[differs] > other[differs]; +}; + export default class Dependencies extends BaseUpgradeStep { type = 'Update the extension dependencies to the latest versions.'; @@ -35,10 +116,17 @@ export default class Dependencies extends BaseUpgradeStep { /** What was removed from tsconfig.json, as TypeScript 4.9 doesn't have it. */ protected removedCompilerOptions: string[] = []; - protected extendsList = false; - protected newerSyntax = false; + /** Constraints left as they were, as they already ask for newer versions. */ + protected keptNewer: string[] = []; + + /** Tooling moved from dependencies to devDependencies. */ + protected movedToDev: string[] = []; + + /** The typings scripts, and the tooling they run, added to package.json. */ + protected addedTypings: string[] = []; + replacements(file: string): Replacement[] { const replacements: Replacement[] = []; @@ -50,7 +138,10 @@ export default class Dependencies extends BaseUpgradeStep { replacements.push(this.updatePackageJson()); break; case 'js/tsconfig.json': - replacements.push(this.updateTsConfig()); + replacements.push(this.updateTsConfig(), this.limitTypes()); + break; + case 'js/tsconfig.test.json': + replacements.push(this.testTypes()); break; default: if (isTypeScript(file)) { @@ -62,7 +153,7 @@ export default class Dependencies extends BaseUpgradeStep { } targets(): string[] { - return ['composer.json', 'js/package.json', 'js/tsconfig.json', 'js/src/**/*.ts', 'js/src/**/*.tsx']; + return ['composer.json', 'js/package.json', 'js/tsconfig.json', 'js/tsconfig.test.json', 'js/src/**/*.ts', 'js/src/**/*.tsx']; } // TypeScript sources are only searched, line by line: parsing them would @@ -94,8 +185,10 @@ export default class Dependencies extends BaseUpgradeStep { `TypeScript ${this.typeScriptWas} was brought down to ^4.9.5: Flarum 2.0 builds and checks its typings with TypeScript 4.9.`, this.removedCompilerOptions.length > 0 && `Removed from js/tsconfig.json, as TypeScript 4.9 does not have them: ${this.removedCompilerOptions.join(', ')}.`, - this.extendsList && 'js/tsconfig.json extends a list of configs, which TypeScript 4.9 cannot read. Extend one, and move the rest into it.', this.newerSyntax && 'Some TypeScript uses syntax 4.9 cannot compile; look for the @TODO comments.', + this.addedTypings.length > 0 && `Added what the reusable frontend workflow runs to build and check typings: ${this.addedTypings.join(', ')}.`, + this.movedToDev.length > 0 && `Moved to devDependencies, as the extension only builds with them: ${this.movedToDev.join(', ')}.`, + this.keptNewer.length > 0 && `Kept, as they already ask for newer versions than Flarum 2.0 needs: ${this.keptNewer.join(', ')}.`, ].filter(Boolean); return [ @@ -106,6 +199,18 @@ export default class Dependencies extends BaseUpgradeStep { ].join('\n '); } + /** + * Require at least the given version, leaving a constraint that already + * asks for a newer one as it is. + */ + protected raise(dependencies: Record, name: string, version: string): void { + if (asksForNewer(String(dependencies[name]), version)) { + this.keptNewer.push(`${name} ${dependencies[name]}`); + } else { + dependencies[name] = version; + } + } + updateComposerJson(): Replacement { return (_file, _code, advanced) => { const composer = advanced as Record; @@ -124,7 +229,7 @@ export default class Dependencies extends BaseUpgradeStep { }); if (key === 'flarum/core') { - composer.require[key] = '^2.0.0'; + this.raise(composer.require, key, '^2.0.0'); } else if (key.startsWith('flarum/') && value !== '*') { composer.require[key] = '*'; } @@ -140,21 +245,22 @@ export default class Dependencies extends BaseUpgradeStep { }); if (['flarum/core', 'flarum/testing', 'flarum/phpstan'].includes(key)) { - composer['require-dev'][key] = '^2.0.0'; + this.raise(composer['require-dev'], key, '^2.0.0'); } else if (key.startsWith('flarum/') && (value as string).startsWith('^1')) { composer['require-dev'][key] = '*'; } } + // An extension using newer PHP than Flarum 2.0 needs keeps asking for it. if (composer.require.php) { - composer.require.php = '^8.3'; + this.raise(composer.require, 'php', '^8.3'); } // Flarum 2.0 is stable, so its constraints resolve without changing the // extension's minimum stability. if (composer.require['fof/extend']) { - composer.require['fof/extend'] = '^2.0.0'; + this.raise(composer.require, 'fof/extend', '^2.0.0'); } return { @@ -187,8 +293,12 @@ export default class Dependencies extends BaseUpgradeStep { this.typeScriptWas = dependencies[key]; } - if (map[key]) { + // TypeScript is the exception: it comes down to what Flarum checks + // typings with, whatever the extension asked for. + if (key === 'typescript') { dependencies[key] = map[key]; + } else if (map[key]) { + this.raise(dependencies, key, map[key]); } } } @@ -197,6 +307,9 @@ export default class Dependencies extends BaseUpgradeStep { advanced.scripts['post-build-typings'] = PORTABLE_POST_BUILD_TYPINGS; } + this.addTypingsScripts(advanced); + this.moveToolingToDevDependencies(advanced); + // Remove duplicates from devDependencies if (advanced.dependencies && advanced.devDependencies) { for (const [key] of Object.entries(advanced.dependencies)) { @@ -212,6 +325,80 @@ export default class Dependencies extends BaseUpgradeStep { }; } + /** + * Add the scripts that build and check typings, which a new extension is + * scaffolded with and the reusable frontend workflow runs, along with the + * tooling they need. Scripts the extension already has are left alone. + */ + protected addTypingsScripts(pkg: Record): void { + const missing = TYPINGS_SCRIPTS.filter((name) => !pkg.scripts?.[name]); + + if (missing.length === 0) return; + + if (!this.hasTsConfig()) { + this.leaveTodo( + 'js/tsconfig.json', + 'Add a tsconfig.json, then the scripts that build and check typings, which the reusable frontend workflow runs: ' + missing.join(', ') + '.' + ); + + return; + } + + const scaffolded = scaffoldedPackageJson(this.packageManager()); + + pkg.scripts = { ...pkg.scripts }; + + for (const name of missing) { + pkg.scripts[name] = scaffolded.scripts[name]; + } + + const listed = { ...pkg.dependencies, ...pkg.devDependencies }; + const tooling = TYPINGS_TOOLING.filter((name) => !listed[name]); + + if (tooling.length > 0) { + pkg.devDependencies = sortedByName({ + ...pkg.devDependencies, + ...Object.fromEntries(tooling.map((name) => [name, scaffolded.devDependencies[name]])), + }); + } + + this.addedTypings.push(...missing, ...tooling); + } + + protected hasTsConfig(): boolean { + return fs.existsSync(path.join(this.root, 'js/tsconfig.json')); + } + + /** The package manager the extension's frontend uses, by its lockfile. */ + protected packageManager(): 'yarn' | 'npm' { + return fs.existsSync(path.join(this.root, 'js/yarn.lock')) ? 'yarn' : 'npm'; + } + + /** + * Flarum 2.0 extensions only build with their tooling: none of it ends up + * in the bundle, so it belongs in devDependencies. Where the extension lists + * a tool under both keys, the newer constraint is kept. Other dependencies + * may be libraries the bundle includes, so they stay where they are. + */ + protected moveToolingToDevDependencies(pkg: Record): void { + const moving = Object.keys(pkg.dependencies || {}).filter((name) => BUILD_TOOLING.has(name) || name.startsWith('@types/')); + + if (moving.length === 0) return; + + const devDependencies: Record = { ...pkg.devDependencies }; + + for (const name of moving) { + if (!devDependencies[name] || asksForNewer(String(pkg.dependencies[name]), String(devDependencies[name]))) { + devDependencies[name] = pkg.dependencies[name]; + } + + delete pkg.dependencies[name]; + } + + pkg.devDependencies = sortedByName(devDependencies); + this.movedToDev.push(...moving); + } + /** * Drop the compiler options, and option values, TypeScript 4.9 doesn't have: * it refuses a tsconfig.json with any. A tsconfig.json it can read is left @@ -247,13 +434,72 @@ export default class Dependencies extends BaseUpgradeStep { } } - this.extendsList = this.extendsList || Array.isArray(tsconfig.extends); + if (Array.isArray(tsconfig.extends)) { + this.leaveTodo('js/tsconfig.json', 'It extends a list of configs, which TypeScript 4.9 cannot read. Extend one, and move the rest into it.'); + } + this.removedCompilerOptions.push(...removed); return removed.length > 0 ? { updated: tsconfig } : null; }; } + /** + * TypeScript loads every type package in node_modules unless told which, + * and 4.9 can't read the @types/node that webpack's minimizer brings along. + * Load only the ones the code imports. The file is edited as text so its + * comments stay. + */ + limitTypes(): Replacement { + return (_file, code, advanced) => { + if ((advanced as Record)?.compilerOptions?.types !== undefined) return null; + + const updated = insertCompilerOption(code, TYPES_COMMENT, '"types": []'); + + if (!updated) return null; + + const globals = Object.keys({ ...this.packageJson().dependencies, ...this.packageJson().devDependencies }).filter( + (name) => name.startsWith('@types/') && name !== '@types/jest' + ); + + if (globals.length > 0) { + this.leaveTodo( + 'js/tsconfig.json', + `It only loads the type packages the code imports now. If the code uses the globals of ${globals.join(', ')}, list them in compilerOptions.types.` + ); + } + + return { updated }; + }; + } + + /** + * A test config extending tsconfig.json would no longer load Jest's globals. + */ + testTypes(): Replacement { + return (_file, code, advanced) => { + if ((advanced as Record)?.compilerOptions?.types !== undefined) return null; + + const pkg = this.packageJson(); + const listed = { ...pkg.dependencies, ...pkg.devDependencies }; + + if (!['@flarum/jest-config', 'jest', '@types/jest'].some((name) => listed[name])) return null; + + const updated = insertCompilerOption(code, null, '"types": ["jest"]'); + + return updated ? { updated } : null; + }; + } + + /** The extension's js/package.json, or nothing when it has none. */ + protected packageJson(): Record { + try { + return JSON.parse(fs.readFileSync(path.join(this.root, 'js/package.json'), 'utf8')); + } catch { + return {}; + } + } + /** * Put a @TODO above each line using syntax TypeScript 4.9 can't compile. */ diff --git a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts index 5e34a25c..a967ee73 100644 --- a/src/steps/upgrade/twopointoh/frontend/admin-extender.ts +++ b/src/steps/upgrade/twopointoh/frontend/admin-extender.ts @@ -909,9 +909,28 @@ function indexEdits( ): string { const edits: Edit[] = []; const moved = new Set(registrations.map((registration) => registration.statement.node)); + const emptied = emptiedInitializers(registrations, moved); + + // An initializer left with nothing to do goes as a whole. 2.0 checks + // whether an extension is enabled in flarum.extensions, not by its + // initializer, so nothing needs it to stay. + // The app it added itself to may now go unused too. + const candidates = new Map(imports); + + for (const initializer of emptied) { + moved.add(initializer.node); + edits.push(removal(code, initializer)); + + const app = initializer.get('expression.callee.object.object') as NodePath; + const binding = app.isIdentifier() ? app.scope.getBinding(app.node.name) : undefined; + + if (binding && isImport(binding)) candidates.set(binding.identifier.name, binding); + } for (const registration of registrations) { - edits.push(removal(code, registration)); + if (!emptied.some((initializer) => registration.statement.isDescendant(initializer))) { + edits.push(removal(code, registration.statement)); + } } // A constant only the moved code, or another constant going, used goes too. @@ -970,7 +989,7 @@ function indexEdits( !reference.findParent((parent) => moved.has(parent.node) || [...going].some((g) => g.path.parentPath!.node === parent.node)); const byDeclaration = new Map(); - for (const binding of imports.values()) { + for (const binding of candidates.values()) { if (references(program, binding).some(outside)) continue; const declaration = binding.path.parent as t.ImportDeclaration; @@ -1025,10 +1044,35 @@ function references(program: NodePath, binding: Binding): NodePath[] return found; } +/** + * The `app.initializers.add(...)` statements that only held registrations + * which are moving, and no comments of their own. + */ +function emptiedInitializers(registrations: Registration[], moved: Set): NodePath[] { + const emptied: NodePath[] = []; + + for (const registration of registrations) { + const block = registration.statement.parentPath as NodePath; + const statement = block.parentPath?.parentPath?.parentPath; + + if (!statement?.isExpressionStatement() || emptied.some((other) => other.node === statement.node)) continue; + + const body = block.node.body; + const removed = new Set(body.flatMap((node) => node.leadingComments ?? [])); + const kept = [...(block.node.innerComments ?? []), ...(body.at(-1)?.trailingComments ?? [])].filter((comment) => !removed.has(comment)); + + if (body.every((node) => moved.has(node)) && kept.length === 0) { + emptied.push(statement); + } + } + + return emptied; +} + /** The statement's lines, with the comments above it and one ending its line. */ -function removal(code: string, registration: Registration): Edit { - const node = registration.statement.node; - const parent = registration.statement.parentPath.node as t.BlockStatement; +function removal(code: string, statement: NodePath): Edit { + const node = statement.node; + const parent = statement.parentPath!.node as t.BlockStatement | t.Program; const previous = parent.body[parent.body.indexOf(node) - 1]; const leading = (node.leadingComments ?? []).filter((comment) => !previous || comment.loc!.start.line > previous.loc!.end.line); const start = leading.length > 0 ? leading[0].start! : node.start!; 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 2adb7370..fc23bd5d 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 @@ -8,13 +8,14 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { type = 'Adapt to accessing/extending lazy loaded Flarum modules.'; // Array.from(flarum.reg.chunks.entries().flatMap(e => e[1].modules.map(m => e[1].namespace + ':' + m))).join('\n') + // LabelValue and LoadingPost were lazy once, and are in the main bundle + // now (flarum/framework#4235), so they are imported as usual. // core:forum/components/ReplyComposer // core:forum/components/ComposerBody // core:common/components/ConfirmDocumentUnload // core:forum/components/LogInModal // core:forum/components/ForgotPasswordModal // core:forum/components/PostStream - // core:forum/components/LoadingPost // core:forum/components/ReplyPlaceholder // core:forum/components/PostStreamScrubber // core:forum/components/SearchModal @@ -33,7 +34,6 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { // core:forum/components/AccessTokensList // core:forum/components/NewAccessTokenModal // core:forum/states/UserSecurityPageState - // core:common/components/LabelValue // core:forum/components/NotificationsPage // core:forum/components/Composer // core:forum/components/ComposerButton @@ -51,15 +51,11 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { 'ForgotPasswordModal', 'NotificationsPage', 'ComposerBody', - 'ConfirmDocumentUnload', - 'LoadingPost', 'ReplyPlaceholder', 'PostStreamScrubber', - 'SearchModal', 'SignUpModal', 'DiscussionComposer', 'EditPostComposer', - 'ComposerBody', 'LogInModal', 'PostStream', 'ReplyComposer', @@ -73,16 +69,16 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { 'UserSecurityPageState', 'ComposerButton', ], - common: ['EditUserModal', 'LabelValue', 'ConfirmDocumentUnload'], + common: ['EditUserModal', 'ConfirmDocumentUnload', 'SearchModal'], }, 'flarum/tags': { admin: [], - forum: ['TagDiscussionModal', 'TagSelectionModal', 'ToggleButton'], - common: [], + forum: ['TagDiscussionModal', 'ToggleButton'], + common: ['TagSelectionModal'], }, 'flarum/emoji': { admin: [], - forum: ['EmojiMap'], + forum: ['emojiMap'], common: [], }, current: { @@ -424,11 +420,33 @@ export default class UsageOfLazyModules extends BaseUpgradeStep { removeImportLines(): Replacement { return (_file, code) => { - this.forEachLazyModule(code, ({ module, importLine }) => { + this.forEachLazyModule(code, ({ module, importPath, importedAs, importLine }) => { if (code.includes(` extends ${module}`)) { return; } + // Types are erased from the build, so a type import costs nothing. + if (/^type\s/.test(importedAs.trim())) { + return; + } + + // The other replacements load what they can as it's needed. Whatever + // still uses the import, such as JSX rendering the component, would + // break without it, and only the author knows where to load it. A name + // inside a string, such as a module path, is no use of it. + const names = importedAs.match(/[$A-Z_a-z][\w$]*/g)?.filter((name) => name !== 'as' && name !== 'default') ?? []; + const rest = code.replace(importLine, ''); + + if (names.some((name) => new RegExp(`(? { + if (/["']flarum\/forum\/components\/AvatarEditor["']/.test(code)) { + this.leaveTodo( + file, + 'It uses AvatarEditor, which in 2.0 opens the file picker, or uploads a dropped image, whenever user.hasUploadedAvatar() is false, not when there is no avatarUrl(). Check what it does for users without an uploaded avatar.' + ); + } + + return null; + }; + } + targets(): string[] { return ['js/src/**/*']; } diff --git a/src/steps/upgrade/twopointoh/infra.ts b/src/steps/upgrade/twopointoh/infra.ts index 13a28f86..8eb97eb6 100644 --- a/src/steps/upgrade/twopointoh/infra.ts +++ b/src/steps/upgrade/twopointoh/infra.ts @@ -1,4 +1,10 @@ -import { BaseUpgradeStep, GitCommit, Replacement } from './base'; +import simpleGit from 'simple-git'; +import { AdvancedContent, BaseUpgradeStep, GitCommit, Replacement } from './base'; + +// Where 1.x extensions got core's global typings from, and where 2.x ones get +// the global typings of every Flarum package they depend on. +const CORE_GLOBAL_TYPINGS = '"../vendor/flarum/core/js/dist-typings/@types/**/*"'; +const VENDOR_GLOBAL_TYPINGS = '"../vendor/*/*/js/dist-typings/@types/**/*"'; const indentation = (line: string): number => line.length - line.trimStart().length; @@ -48,11 +54,21 @@ export default class Infrastructure extends BaseUpgradeStep { replacements.push(this.updatePhpStan()); } + if (file === 'js/tsconfig.json') { + replacements.push(this.updateTsConfigInclude()); + } + return replacements; } targets(): string[] { - return ['.github/workflows/*.yml', 'phpstan.neon']; + return ['.github/workflows/*.yml', 'phpstan.neon', 'js/tsconfig.json']; + } + + // Every file is only edited as text: a tsconfig.json the JSON parser can't + // read shouldn't stop the workflows being updated. + advancedContent(_file: string, _code: string): AdvancedContent { + return null; } gitCommit(): GitCommit { @@ -67,17 +83,66 @@ export default class Infrastructure extends BaseUpgradeStep { } updateWorkflow(): Replacement { - return (_file, code) => ({ - updated: dropPhpVersions( - code - .replace('REUSABLE_backend.yml@1.x', 'REUSABLE_backend.yml@2.x') - .replace('REUSABLE_frontend.yml@1.x', 'REUSABLE_frontend.yml@2.x') - .replace('REUSABLE_backend.yml@main', 'REUSABLE_backend.yml@2.x') - .replace('REUSABLE_frontend.yml@main', 'REUSABLE_frontend.yml@2.x') + return async (_file, code) => ({ + updated: await this.updateMainGitBranch( + dropPhpVersions( + code + .replace('REUSABLE_backend.yml@1.x', 'REUSABLE_backend.yml@2.x') + .replace('REUSABLE_frontend.yml@1.x', 'REUSABLE_frontend.yml@2.x') + .replace('REUSABLE_backend.yml@main', 'REUSABLE_backend.yml@2.x') + .replace('REUSABLE_frontend.yml@main', 'REUSABLE_frontend.yml@2.x') + ) ), }); } + /** + * The reusable frontend workflow commits the built JS to `main_git_branch`, + * which still names 1.x. Point it at a 2.x branch when there is one. Other + * values are left alone: an extension's 2.x code is often on main. + */ + async updateMainGitBranch(code: string): Promise { + const setting = /^([\t ]*)main_git_branch:([\t ]*)(["']?)1\.x\3/m; + const match = code.match(setting); + + if (!match) return code; + + if (await this.hasBranch('2.x')) { + return code.replace(setting, (_line, indent, space, quote) => `${indent}main_git_branch:${space}${quote}2.x${quote}`); + } + + const todo = `${match[1]}# @TODO: Set main_git_branch to the branch your Flarum 2.x code is on. The built JS is committed to it.`; + + // An earlier run already left it. + if (code.includes(todo)) return code; + + return code.replace(setting, (line) => `${todo}\n${line}`); + } + + /** Whether the extension's repository has the branch, locally or on a remote. */ + protected async hasBranch(name: string): Promise { + try { + const branches = await simpleGit(this.root).branch(['--all']); + + return branches.all.some((branch) => branch === name || (branch.startsWith('remotes/') && branch.endsWith(`/${name}`))); + } catch { + return false; + } + } + + /** + * Read the global typings of every Flarum package in vendor, as a new + * extension does, not only core's. The file is edited as text so its + * comments stay. + */ + updateTsConfigInclude(): Replacement { + return (_file, code) => { + if (!code.includes(CORE_GLOBAL_TYPINGS) || code.includes(VENDOR_GLOBAL_TYPINGS)) return null; + + return { updated: code.replace(CORE_GLOBAL_TYPINGS, VENDOR_GLOBAL_TYPINGS) }; + }; + } + updatePhpStan(): Replacement { return (_file, code) => ({ updated: code diff --git a/src/steps/upgrade/twopointoh/todos.ts b/src/steps/upgrade/twopointoh/todos.ts index 5de98ef6..1d3a65d7 100644 --- a/src/steps/upgrade/twopointoh/todos.ts +++ b/src/steps/upgrade/twopointoh/todos.ts @@ -7,7 +7,8 @@ export const TODO_MARKER = '@TODO'; export type Todo = { file: string; - line: number; + /** Null for work a step leaves without a @TODO comment in the file to show for it. */ + line: number | null; text: string; }; diff --git a/test/steps/upgrade/admin-extender.test.ts b/test/steps/upgrade/admin-extender.test.ts index 13a82cb0..e3981f90 100644 --- a/test/steps/upgrade/admin-extender.test.ts +++ b/test/steps/upgrade/admin-extender.test.ts @@ -55,13 +55,10 @@ app.initializers.add('acme-interstellar', () => { `, }); - // The initializer stays: other extensions may check for it by name. + // The initializer has nothing left to do, so it goes, and the app only it + // used. 2.0 checks whether an extension is enabled in flarum.extensions. expect(index).toBe( - await format(`import app from 'flarum/admin/app'; - -export { default as extend } from './extend'; - -app.initializers.add('acme-interstellar', () => {}); + await format(`export { default as extend } from './extend'; `) ); @@ -80,6 +77,62 @@ export default [ }); }); + 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'; + +app.initializers.add('acme-interstellar', () => { + app.extensionData.for('acme-interstellar').registerSetting({ setting: 'acme-interstellar.fuel', type: 'text' }); + + console.log('ready'); +}); +`, + }); + + expect(index).toBe( + await format(`import app from 'flarum/admin/app'; + +export { default as extend } from './extend'; + +app.initializers.add('acme-interstellar', () => { + console.log('ready'); +}); +`) + ); + }); + + it('leaving an initializer with a comment of its own in place', async () => { + const { index } = await upgrade({ + 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; + +app.initializers.add('acme-interstellar', () => { + app.extensionData.for('acme-interstellar').registerSetting({ setting: 'acme-interstellar.fuel', type: 'text' }); + // More settings once the launch pad is built. +}); +`, + }); + + expect(index).toContain("app.initializers.add('acme-interstellar'"); + expect(index).toContain('// More settings once the launch pad is built.'); + }); + + it('removing an initializer with the comments above it', async () => { + const { index } = await upgrade({ + 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; + +// Register the settings. +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'; +`) + ); + }); + it('each registration as its Admin extender method, with its priority and type', async () => { const { index, written } = await upgrade({ 'js/src/admin/index.ts': `import app from 'flarum/admin/app'; @@ -342,8 +395,6 @@ app.initializers.add('acme-interstellar', (app) => { expect(index).toBe( await format(`export { default as extend } from './extend'; - -app.initializers.add('acme-interstellar', (app) => {}); `) ); @@ -452,11 +503,7 @@ export default [...commonExtend]; }); expect(index).toBe( - await format(`import app from 'flarum/admin/app'; - -export { default as extend } from './extend'; - -app.initializers.add('acme-interstellar', () => {}); + await format(`export { default as extend } from './extend'; `) ); @@ -573,11 +620,7 @@ app.initializers.add('acme-interstellar', () => { }); expect(index).toBe( - await format(`import app from 'flarum/admin/app'; - -export { default as extend } from './extend'; - -app.initializers.add('acme-interstellar', () => {}); + await format(`export { default as extend } from './extend'; `) ); diff --git a/test/steps/upgrade/dependencies.test.ts b/test/steps/upgrade/dependencies.test.ts index 95d1a90f..0313a2de 100644 --- a/test/steps/upgrade/dependencies.test.ts +++ b/test/steps/upgrade/dependencies.test.ts @@ -52,6 +52,21 @@ const onePackageJson = { }, }; +/** + * The package.json as the upgrade writes it back, for an extension with or + * without a tsconfig.json, using the given package manager. + */ +async function withTypings(content: Record, { tsconfig = true, packageManager = 'yarn' } = {}) { + const step = new Dependencies({} as any, '/extension', 1, 1, false); + jest.spyOn(step as any, 'hasTsConfig').mockReturnValue(tsconfig); + jest.spyOn(step as any, 'packageManager').mockReturnValue(packageManager); + + const code = JSON.stringify(content, null, 2); + const result = await step.applyReplacements('js/package.json', code, step.advancedContent('js/package.json', code)); + + return { pkg: JSON.parse(result.updated as string), notes: step.pauseMessage(), todos: (step as any).followUps }; +} + describe('the dependencies step', () => { describe('in composer.json', () => { it('requires Flarum 2.0 and PHP 8.3', async () => { @@ -70,6 +85,29 @@ describe('the dependencies step', () => { expect(composer).not.toHaveProperty('prefer-stable'); }); + it('keeps a PHP constraint newer than Flarum 2.0 needs', async () => { + const composer = await upgrade('composer.json', { ...oneComposerJson, require: { ...oneComposerJson.require, php: '^8.4' } }); + + expect(composer.require.php).toBe('^8.4'); + }); + + it('keeps a Flarum constraint that already asks for a later 2.x release', async () => { + const composer = await upgrade('composer.json', { + require: { 'flarum/core': '^2.0.1' }, + 'require-dev': { 'flarum/testing': '^2.0.1', 'flarum/phpstan': '^2.0.0' }, + }); + + expect(composer.require['flarum/core']).toBe('^2.0.1'); + expect(composer['require-dev']['flarum/testing']).toBe('^2.0.1'); + }); + + it('still replaces a constraint it cannot compare', async () => { + const composer = await upgrade('composer.json', { require: { 'flarum/core': '*' }, 'require-dev': { 'flarum/core': '*@dev' } }); + + expect(composer.require['flarum/core']).toBe('^2.0.0'); + expect(composer['require-dev']['flarum/core']).toBe('^2.0.0'); + }); + it('leaves the stability settings the extension already has', async () => { const composer = await upgrade('composer.json', { ...oneComposerJson, 'minimum-stability': 'dev', 'prefer-stable': true }); @@ -82,7 +120,7 @@ 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.dependencies['flarum-webpack-config']).toBe('^3.0.6'); + expect(pkg.devDependencies['flarum-webpack-config']).toBe('^3.0.6'); expect(pkg.devDependencies['flarum-tsconfig']).toBe('^2.0.0'); }); @@ -95,17 +133,71 @@ describe('the dependencies step', () => { it('requires the webpack Flarum builds with', async () => { const pkg = await upgrade('js/package.json', onePackageJson); - expect(pkg.dependencies.webpack).toBe('^5.104.1'); + expect(pkg.devDependencies.webpack).toBe('^5.104.1'); + }); + + it('keeps tooling the extension already asks for newer versions of, and says so', async () => { + const step = new Dependencies({} as any, '/extension', 1, 1, false); + const code = JSON.stringify({ devDependencies: { 'webpack-cli': '^5.1.4', webpack: '^5.65.0' } }, null, 2); + const result = await step.applyReplacements('js/package.json', code, step.advancedContent('js/package.json', code)); + const pkg = JSON.parse(result.updated as string); + + expect(pkg.devDependencies['webpack-cli']).toBe('^5.1.4'); + expect(pkg.devDependencies.webpack).toBe('^5.104.1'); + expect(step.pauseMessage()).toContain('webpack-cli ^5.1.4'); + }); + + it('moves the build tooling to devDependencies, and says so', async () => { + const step = new Dependencies({} as any, '/extension', 1, 1, false); + const code = JSON.stringify(onePackageJson, null, 2); + const result = await step.applyReplacements('js/package.json', code, step.advancedContent('js/package.json', code)); + const pkg = JSON.parse(result.updated as string); + + expect(pkg.dependencies).toEqual({}); + expect(Object.keys(pkg.devDependencies)).toEqual([ + '@flarum/prettier-config', + 'flarum-tsconfig', + 'flarum-webpack-config', + 'prettier', + 'webpack', + 'webpack-cli', + ]); + expect(step.pauseMessage()).toContain( + 'Moved to devDependencies, as the extension only builds with them: flarum-webpack-config, webpack, webpack-cli.' + ); + }); + + it('moves type packages to devDependencies', async () => { + const pkg = await upgrade('js/package.json', { dependencies: { '@types/mithril': '^2.0.8' } }); + + expect(pkg.devDependencies['@types/mithril']).toBe('^2.0.8'); + expect(pkg.dependencies).not.toHaveProperty('@types/mithril'); + }); + + it('leaves the libraries the bundle may include in dependencies', async () => { + const pkg = await upgrade('js/package.json', { dependencies: { 'flarum-webpack-config': '^2.0.0', 'emoji-mart': '^5.6.0' } }); + + expect(pkg.dependencies).toEqual({ 'emoji-mart': '^5.6.0' }); }); - it('drops a devDependency the extension also lists as a dependency', async () => { + it('keeps the newer constraint of tooling listed under both keys', async () => { const pkg = await upgrade('js/package.json', { - dependencies: { 'flarum-webpack-config': '^2.0.0' }, + dependencies: { 'flarum-webpack-config': '^2.0.0', prettier: '^3.0.3' }, devDependencies: { 'flarum-webpack-config': '^2.0.0', prettier: '^2.5.1' }, }); - expect(pkg.dependencies['flarum-webpack-config']).toBe('^3.0.6'); - expect(pkg.devDependencies).not.toHaveProperty('flarum-webpack-config'); + expect(pkg.dependencies).toEqual({}); + expect(pkg.devDependencies).toEqual({ 'flarum-webpack-config': '^3.0.6', prettier: '^3.0.3' }); + }); + + it('still drops a devDependency the extension also lists as a dependency, when it is not tooling', async () => { + const pkg = await upgrade('js/package.json', { + dependencies: { 'emoji-mart': '^5.6.0' }, + devDependencies: { 'emoji-mart': '^5.0.0', prettier: '^2.5.1' }, + }); + + expect(pkg.dependencies['emoji-mart']).toBe('^5.6.0'); + expect(pkg.devDependencies).not.toHaveProperty('emoji-mart'); }); it('handles a package.json without dependencies', async () => { @@ -128,6 +220,70 @@ describe('the dependencies step', () => { }); }); + describe('the scripts that build and check typings', () => { + const typings = ['clean-typings', 'build-typings', 'post-build-typings', 'check-typings', 'check-typings-coverage']; + + it('adds the ones a new extension is scaffolded with, and what they run', async () => { + const scaffolded = scaffoldedPackageJson(); + const { pkg, notes } = await withTypings(onePackageJson); + + for (const name of typings) { + expect([name, pkg.scripts[name]]).toEqual([name, scaffolded.scripts[name].replace(/<%= params\.jsPackageManager %>/g, 'yarn')]); + } + + expect(pkg.devDependencies.typescript).toBe('^4.9.5'); + expect(pkg.devDependencies['typescript-coverage-report']).toBe(scaffolded.devDependencies['typescript-coverage-report']); + expect(notes).toContain( + 'build and check typings: clean-typings, build-typings, post-build-typings, check-typings, check-typings-coverage, typescript, typescript-coverage-report.' + ); + }); + + it('runs them with the package manager the extension uses', async () => { + const { pkg } = await withTypings(onePackageJson, { packageManager: 'npm' }); + + expect(pkg.scripts['build-typings']).toContain('npm run clean-typings'); + expect(pkg.scripts['build-typings']).not.toContain('yarn'); + }); + + it('keeps the scripts the extension already has', async () => { + const { pkg } = await withTypings({ ...onePackageJson, scripts: { build: 'webpack --mode production', 'check-typings': 'tsc --noEmit' } }); + + expect(pkg.scripts.build).toBe('webpack --mode production'); + expect(pkg.scripts['check-typings']).toBe('tsc --noEmit'); + expect(pkg.scripts).toHaveProperty('check-typings-coverage'); + }); + + it('keeps the TypeScript the extension lists, wherever it lists it', async () => { + const { pkg } = await withTypings({ dependencies: { typescript: '^4.5.4' } }); + + expect(pkg.devDependencies.typescript).toBe('^4.9.5'); + expect(pkg.dependencies).not.toHaveProperty('typescript'); + }); + + it('adds nothing to an extension that has them all', async () => { + const scaffolded = scaffoldedPackageJson(); + const scripts = Object.fromEntries(typings.map((name) => [name, scaffolded.scripts[name]])); + const { pkg, notes } = await withTypings({ scripts, devDependencies: { prettier: '^2.5.1' } }); + + expect(pkg.devDependencies).toEqual({ prettier: '^2.5.1' }); + expect(notes).not.toContain('build and check typings'); + }); + + it('adds none without a tsconfig.json to check against, and leaves that to do', async () => { + const { pkg, todos } = await withTypings(onePackageJson, { tsconfig: false }); + + expect(pkg.scripts).toBeUndefined(); + expect(pkg.devDependencies).not.toHaveProperty('typescript'); + expect(todos).toEqual([ + { + file: 'js/tsconfig.json', + line: null, + text: 'Add a tsconfig.json, then the scripts that build and check typings, which the reusable frontend workflow runs: clean-typings, build-typings, post-build-typings, check-typings, check-typings-coverage.', + }, + ]); + }); + }); + describe('a new extension', () => { it('is scaffolded with flarum-webpack-config 3.0.6 and the webpack Flarum builds with', () => { const scaffolded = scaffoldedPackageJson().devDependencies; diff --git a/test/steps/upgrade/infra.test.ts b/test/steps/upgrade/infra.test.ts index c0fd4e7d..0f3fd211 100644 --- a/test/steps/upgrade/infra.test.ts +++ b/test/steps/upgrade/infra.test.ts @@ -3,8 +3,9 @@ import Infrastructure from '../../../src/steps/upgrade/twopointoh/infra'; /** * The workflow as the upgrade writes it back. */ -async function upgrade(code: string, file = '.github/workflows/backend.yml'): Promise { +async function upgrade(code: string, file = '.github/workflows/backend.yml', branches: string[] = []): Promise { const step = new Infrastructure({} as any, '/extension', 1, 1, false); + jest.spyOn(step as any, 'hasBranch').mockImplementation(async (name) => branches.includes(name as string)); const result = await step.applyReplacements(file, code, step.advancedContent(file, code)); return result.updated as string; @@ -98,4 +99,92 @@ jobs: expect(await upgrade(frontend, '.github/workflows/frontend.yml')).toBe(frontend.replace('@1.x', '@2.x')); }); + + describe('main_git_branch, the branch the built JS is committed to', () => { + const frontend = `jobs: + run: + uses: flarum/framework/.github/workflows/REUSABLE_frontend.yml@1.x + with: + frontend_directory: ./js + main_git_branch: 1.x +`; + + it('moves from 1.x to the 2.x branch', async () => { + const workflow = await upgrade(frontend, '.github/workflows/frontend.yml', ['1.x', '2.x']); + + expect(workflow).toContain(' main_git_branch: 2.x\n'); + expect(workflow).not.toContain('@TODO'); + }); + + it('keeps the quotes around it', async () => { + const workflow = await upgrade(frontend.replace('main_git_branch: 1.x', "main_git_branch: '1.x'"), '.github/workflows/frontend.yml', ['2.x']); + + expect(workflow).toContain(" main_git_branch: '2.x'\n"); + }); + + it('asks for the branch when there is no 2.x branch to point it at', async () => { + const workflow = await upgrade(frontend, '.github/workflows/frontend.yml', ['1.x', 'main']); + + expect(workflow).toContain( + ' # @TODO: Set main_git_branch to the branch your Flarum 2.x code is on. The built JS is committed to it.\n main_git_branch: 1.x\n' + ); + }); + + it('asks only once when run again', async () => { + const once = await upgrade(frontend, '.github/workflows/frontend.yml'); + + expect(await upgrade(once, '.github/workflows/frontend.yml')).toBe(once); + }); + + it('leaves main alone, as 2.x code is often there', async () => { + const onMain = frontend.replace('main_git_branch: 1.x', 'main_git_branch: main'); + + expect(await upgrade(onMain, '.github/workflows/frontend.yml', ['2.x'])).toBe(onMain.replace('@1.x', '@2.x')); + }); + }); + + describe('js/tsconfig.json', () => { + // The tsconfig.json `fl init` gave a 1.x extension. + const oneTsConfig = `{ + // Use Flarum's tsconfig as a starting point + "extends": "flarum-tsconfig", + // This will match all .ts, .tsx, .d.ts, .js, .jsx files in your \`src\` folder + // and also tells your Typescript server to read core's global typings for + // access to \`dayjs\` and \`$\` in the global namespace. + "include": ["src/**/*", "../vendor/flarum/core/js/dist-typings/@types/**/*"], + "compilerOptions": { + // This will output typings to \`dist-typings\` + "declarationDir": "./dist-typings", + "baseUrl": ".", + "paths": { + "flarum/*": ["../vendor/flarum/core/js/dist-typings/*"] + } + } +} +`; + + it('reads the global typings of every Flarum package in vendor, keeping the comments', async () => { + const tsconfig = await upgrade(oneTsConfig, 'js/tsconfig.json'); + + expect(tsconfig).toBe( + oneTsConfig.replace('"../vendor/flarum/core/js/dist-typings/@types/**/*"', '"../vendor/*/*/js/dist-typings/@types/**/*"') + ); + }); + + it('does not add the root @types folder, which would move the built typings', async () => { + expect(await upgrade(oneTsConfig, 'js/tsconfig.json')).not.toContain('"@types/**/*"'); + }); + + it('leaves one that already reads them all as it was', async () => { + const both = oneTsConfig.replace('"src/**/*",', '"src/**/*", "../vendor/*/*/js/dist-typings/@types/**/*",'); + + expect(await upgrade(both, 'js/tsconfig.json')).toBe(both); + }); + + it('does not parse it, so one the JSON parser cannot read is no failure', async () => { + const step = new Infrastructure({} as any, '/extension', 1, 1, false); + + expect(step.advancedContent('js/tsconfig.json', '{ /* a block comment */ }')).toBeNull(); + }); + }); }); diff --git a/test/steps/upgrade/json-api.test.ts b/test/steps/upgrade/json-api.test.ts new file mode 100644 index 00000000..8d803297 --- /dev/null +++ b/test/steps/upgrade/json-api.test.ts @@ -0,0 +1,69 @@ +import path from 'path'; +import { PhpSubsystemProvider } from '../../../src/providers/php-provider'; +import JsonApi from '../../../src/steps/upgrade/twopointoh/backend/json-api'; + +const php = new PhpSubsystemProvider(path.resolve(__dirname, '../../../php-subsystem/index.php')); + +const upgrade = (code: string): string => php.run('upgrade.2-0.json-api', { file: 'extend.php', code }).code; + +// The extend.php of a 1.x extension adding an attribute to a core serializer. +const oneExtendPhp = `attributes(Api\\AddThing::class), +]; +`; + +describe('the JSON:API step, in extend.php', () => { + it('marks the extenders to replace with a TODO linking to the docs', () => { + expect(upgrade(oneExtendPhp)).toContain( + ' // @TODO: Replace with the new implementation https://docs.flarum.org/2.x/extend/api#extending-api-resources\n (new Extend\\ApiSerializer(BasicUserSerializer::class))' + ); + }); + + it('adds no imports, as nothing uses them until the TODO is done', () => { + expect(upgrade(oneExtendPhp)).toContain(` +use Flarum\\Api\\Serializer\\BasicUserSerializer; +use Flarum\\Extend; + +return [`); + }); + + it('leaves an extend.php without API extenders as it was', () => { + const code = oneExtendPhp.replace( + ` (new Extend\\ApiSerializer(BasicUserSerializer::class)) + ->attributes(Api\\AddThing::class), +`, + '' + ); + + expect(upgrade(code)).not.toContain('@TODO'); + }); +}); + +describe('the JSON:API step, on code that sets avatarUrl', () => { + const followUps = async (code: string) => { + const step = new JsonApi({} as any, '/extension', 1, 1, false); + + await step.flagAvatarUrl()('src/Api/AddGravatar.php', code, null); + + return (step as any).followUps; + }; + + it('leaves setting avatarSrcset to do, which browsers prefer', async () => { + // As ianm/gravatar's 1.x serializer attributes have it. + expect(await followUps("gravatar->getForUser($user->id);\n")).toEqual([ + { file: 'src/Api/AddGravatar.php', line: null, text: expect.stringContaining('avatarSrcset, which browsers prefer over the URL') }, + ]); + }); + + it('leaves nothing to do for code that does not', async () => { + expect(await followUps("display_name;\n")).toEqual([]); + }); +}); diff --git a/test/steps/upgrade/json-layout.test.ts b/test/steps/upgrade/json-layout.test.ts new file mode 100644 index 00000000..23a59d79 --- /dev/null +++ b/test/steps/upgrade/json-layout.test.ts @@ -0,0 +1,47 @@ +import Dependencies from '../../../src/steps/upgrade/twopointoh/dependencies'; + +/** + * The file as a step writes it back after changing `flarum/core`. + */ +async function writeBack(file: string, code: string): Promise { + const step = new Dependencies({} as any, '/extension', 1, 1, false); + const content = step.advancedContent(file, code) as Record; + + content.require['flarum/core'] = '^2.0.0'; + + return step.updateCode(file, code, content); +} + +describe('a JSON file written back by an upgrade step', () => { + it('keeps its trailing newline', async () => { + const code = '{\n "require": {\n "flarum/core": "^1.2.0"\n }\n}\n'; + + expect(await writeBack('composer.json', code)).toBe('{\n "require": {\n "flarum/core": "^2.0.0"\n }\n}\n'); + }); + + it('gets no trailing newline when it had none', async () => { + const code = '{\n "require": {\n "flarum/core": "^1.2.0"\n }\n}'; + + expect(await writeBack('composer.json', code)).toBe('{\n "require": {\n "flarum/core": "^2.0.0"\n }\n}'); + }); + + it('keeps its indentation', async () => { + const twoSpaces = '{\n "require": {\n "flarum/core": "^1.2.0"\n }\n}\n'; + const tabs = '{\n\t"require": {\n\t\t"flarum/core": "^1.2.0"\n\t}\n}\n'; + + expect(await writeBack('composer.json', twoSpaces)).toBe('{\n "require": {\n "flarum/core": "^2.0.0"\n }\n}\n'); + expect(await writeBack('js/package.json', tabs)).toBe('{\n\t"require": {\n\t\t"flarum/core": "^2.0.0"\n\t}\n}\n'); + }); + + it('keeps Windows line endings', async () => { + const code = '{\r\n "require": {\r\n "flarum/core": "^1.2.0"\r\n }\r\n}\r\n'; + + expect(await writeBack('composer.json', code)).toBe('{\r\n "require": {\r\n "flarum/core": "^2.0.0"\r\n }\r\n}\r\n'); + }); + + it('is indented as before when it has no indented line to go by', async () => { + expect(await writeBack('composer.json', '{"require": {"flarum/core": "^1.2.0"}}')).toBe( + '{\n "require": {\n "flarum/core": "^2.0.0"\n }\n}' + ); + }); +}); diff --git a/test/steps/upgrade/json-report.test.ts b/test/steps/upgrade/json-report.test.ts index e0a21f7b..a64f52cb 100644 --- a/test/steps/upgrade/json-report.test.ts +++ b/test/steps/upgrade/json-report.test.ts @@ -12,6 +12,25 @@ import TwoPointOh from '../../../src/commands/upgrade/2.0'; const INFRASTRUCTURE = 3; const MISC_FRONTEND = 7; +const MISC_BACKEND = 9; +const EMAIL_VIEWS = 12; + +/** A notification blueprint that is emailed with the given views. */ +const mailableBlueprint = (views: string) => ` = {}): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'upgrade-json-')); const files: Record = { @@ -28,6 +47,7 @@ function extension(): string { 'extend.php': ' { expect(step.todos[0].text).toMatch(/^\/\/ @TODO: check whether the extension is enabled/); }); + it('lists the work a step left that no @TODO comment shows, with the file it is about', async () => { + const root = extension({ + 'js/src/forum/components/Avatar.js': "import AvatarEditor from 'flarum/forum/components/AvatarEditor';\n\nexport default AvatarEditor;\n", + }); + + const { report } = await upgrade(root, '--step', String(MISC_FRONTEND)); + + const [step] = report.steps; + + expect(step.status).toBe('committed'); + expect(step.todos).toHaveLength(2); + expect(step.todos[1]).toEqual({ + file: 'js/src/forum/components/Avatar.js', + line: null, + text: expect.stringContaining('user.hasUploadedAvatar()'), + }); + }); + + it('lists the work a step left when it changed nothing', async () => { + const root = extension({ + 'migrations/2021_01_01_000000_widen_coordinates.php': + " function (Builder $schema) {\n $schema->table('acme_coordinates', function (Blueprint $table) {\n $table->string('name', 200)->change();\n });\n },\n];\n", + }); + + const { report } = await upgrade(root, '--step', String(MISC_BACKEND)); + + expect(report.steps[0]).toEqual({ + step: MISC_BACKEND, + title: 'Miscellaneous backend changes.', + status: 'no-changes', + todos: [ + { + file: 'migrations/2021_01_01_000000_widen_coordinates.php', + line: null, + text: expect.stringContaining("repeat the column's whole definition"), + }, + ], + }); + }); + + describe('with --check', () => { + it('reports what a step would change, with its diff', async () => { + const root = extension(); + + const { report, exitCode } = await upgrade(root, '--check', '--step', String(INFRASTRUCTURE)); + + expect(exitCode).toBe(0); + expect(report.check).toEqual({ commit: git(root, 'rev-parse HEAD'), uncommittedChangesLeftOut: false }); + expect(report.steps[0]).toMatchObject({ + step: INFRASTRUCTURE, + status: 'would-change', + files: ['.github/workflows/backend.yml'], + notes: 'Infrastructure updated. Please review the changes then proceed.', + todos: [], + }); + expect(report.steps[0].diff).toContain('+ uses: flarum/framework/.github/workflows/REUSABLE_backend.yml@2.x'); + expect(report.steps[0]).not.toHaveProperty('commit'); + }); + + it('changes nothing in the extension, and leaves no worktree or branch behind', async () => { + const root = extension(); + const before = { head: git(root, 'rev-parse HEAD'), branches: git(root, 'branch --list'), worktrees: git(root, 'worktree list') }; + + await upgrade(root, '--check'); + + expect({ head: git(root, 'rev-parse HEAD'), branches: git(root, 'branch --list'), worktrees: git(root, 'worktree list') }).toEqual(before); + expect(git(root, 'status --porcelain')).toBe(''); + expect(fs.readFileSync(path.join(root, '.github/workflows/backend.yml'), 'utf8')).toContain('REUSABLE_backend.yml@1.x'); + }); + + it('runs steps an earlier upgrade already committed', async () => { + const root = extension(); + + await upgrade(root, '--step', String(INFRASTRUCTURE)); + git(root, 'reset -q --hard HEAD~1'); + git(root, `commit -q --allow-empty -m "chore(2.0): update infrastructure"`); + + const { report } = await upgrade(root, '--check', '--step', String(INFRASTRUCTURE)); + + expect(report.steps[0].status).toBe('would-change'); + }); + + it('checks the last commit of an extension with changes not committed, and says it left them out', async () => { + const root = extension(); + fs.appendFileSync(path.join(root, 'extend.php'), '// local edit\n'); + + const { report, exitCode } = await upgrade(root, '--check', '--step', String(INFRASTRUCTURE)); + + expect(exitCode).toBe(0); + expect(report.check.uncommittedChangesLeftOut).toBe(true); + expect(report.steps[0].status).toBe('would-change'); + expect(fs.readFileSync(path.join(root, 'extend.php'), 'utf8')).toContain('// local edit'); + }); + }); + + describe('email views', () => { + it("wraps a 1.x notification's one view as both, in place of it", async () => { + const root = extension({ + 'src/Notifications/LaunchBlueprint.php': mailableBlueprint("'text' => 'acme-interstellar::emails.launch'"), + 'resources/views/emails/launch.blade.php': 'Liftoff!\n', + }); + + const { report } = await upgrade(root, '--step', String(EMAIL_VIEWS)); + + expect(report.steps[0].status).toBe('committed'); + expect(fs.existsSync(path.join(root, 'resources/views/emails/launch.blade.php'))).toBe(false); + expect(fs.readFileSync(path.join(root, 'resources/views/email/plain/launch.blade.php'), 'utf8')).toContain( + '\n\nLiftoff!' + ); + expect(fs.readFileSync(path.join(root, 'resources/views/email/html/launch.blade.php'), 'utf8')).toContain(''); + expect(fs.readFileSync(path.join(root, 'src/Notifications/LaunchBlueprint.php'), 'utf8')).toContain( + "'text' => 'acme-interstellar::email.plain.launch'" + ); + }); + + it('leaves a notification that has both alone', async () => { + // As fof/follow-users' 2.x notifications have them. + const root = extension({ + 'src/Notifications/LaunchBlueprint.php': mailableBlueprint( + "'text' => 'acme-interstellar::email.plain.launch', 'html' => 'acme-interstellar::email.html.launch'" + ), + 'resources/views/email/plain/launch.blade.php': + '\n\nLiftoff!\n\n\n', + 'resources/views/email/html/launch.blade.php': + '\n \n Liftoff!\n \n\n', + }); + + const { report } = await upgrade(root, '--step', String(EMAIL_VIEWS)); + + expect(report.steps[0].status).toBe('no-changes'); + expect(git(root, 'status --porcelain')).toBe(''); + }); + }); + it('skips a step that already ran', async () => { const root = extension(); diff --git a/test/steps/upgrade/lazy-modules.test.ts b/test/steps/upgrade/lazy-modules.test.ts new file mode 100644 index 00000000..02c83330 --- /dev/null +++ b/test/steps/upgrade/lazy-modules.test.ts @@ -0,0 +1,72 @@ +import UsageOfLazyModules from '../../../src/steps/upgrade/twopointoh/frontend/code-splitting/usage-of-lazy-modules'; + +async function upgrade(code: string, file = 'js/src/forum/index.js'): Promise { + const step = new UsageOfLazyModules({} as any, '/extension', 1, 1, false); + const result = await step.applyReplacements(file, code, step.advancedContent(file, code)); + + return result.updated as string; +} + +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`; + +describe('the lazy modules step, on imports', () => { + it('drops one the code only extends, which is then extended by its path', async () => { + const code = await upgrade(`import { extend } from 'flarum/common/extend'; +import SettingsPage from 'flarum/forum/components/SettingsPage'; + +extend(SettingsPage.prototype, 'settingsItems', function (items) { + items.add('acme', 'Acme'); +}); +`); + + expect(code).not.toContain("import SettingsPage from 'flarum/forum/components/SettingsPage';"); + expect(code).toContain("extend('flarum/forum/components/SettingsPage', 'settingsItems'"); + }); + + it('keeps one the code still uses, and says how to load it', async () => { + const code = await upgrade(`import SignUpModal from 'flarum/forum/components/SignUpModal'; + +export const isSignUp = (modal) => modal instanceof SignUpModal; +`); + + expect(code).toContain(`${todo('SignUpModal', 'flarum/forum/components/SignUpModal')} +import SignUpModal from 'flarum/forum/components/SignUpModal';`); + }); + + it('keeps a type import, which the build erases', async () => { + const code = await upgrade( + `import type LogInModal from 'flarum/forum/components/LogInModal'; + +export function open(modal: LogInModal) {} +`, + 'js/src/forum/index.ts' + ); + + expect(code).toContain("import type LogInModal from 'flarum/forum/components/LogInModal';"); + expect(code).not.toContain('@TODO'); + }); + + it('leaves the modules now in the main bundle alone', async () => { + // As fof/geoip's MapModal has it. + const source = `import LabelValue from 'flarum/common/components/LabelValue'; +import LoadingPost from 'flarum/forum/components/LoadingPost'; + +export const view = () => [, ]; +`; + const code = await upgrade(source, 'js/src/forum/components/MapModal.js'); + + expect(code).toContain("import LabelValue from 'flarum/common/components/LabelValue';"); + expect(code).toContain("import LoadingPost from 'flarum/forum/components/LoadingPost';"); + expect(code).not.toContain('@TODO'); + }); + + it('asks only once when run again', async () => { + const once = await upgrade(`import SignUpModal from 'flarum/forum/components/SignUpModal'; + +export const isSignUp = (modal) => modal instanceof SignUpModal; +`); + + expect((await upgrade(once)).split('@TODO').length).toBe(2); + }); +}); diff --git a/test/steps/upgrade/misc-backend.test.ts b/test/steps/upgrade/misc-backend.test.ts new file mode 100644 index 00000000..1e72c71f --- /dev/null +++ b/test/steps/upgrade/misc-backend.test.ts @@ -0,0 +1,85 @@ +import path from 'path'; +import { PhpSubsystemProvider } from '../../../src/providers/php-provider'; + +const php = new PhpSubsystemProvider(path.resolve(__dirname, '../../../php-subsystem/index.php')); + +const upgrade = (file: string, code: string): string => php.run('upgrade.2-0.misc', { file, code }).code; + +describe('the misc backend step', () => { + describe('notification types', () => { + const extendPhp = (types: string) => ` { + const code = upgrade( + 'extend.php', + extendPhp(` ->type(Notifications\\LaunchBlueprint::class, Api\\LaunchSerializer::class, ['alert', 'email'])`) + ); + + expect(code).toContain("->type(Notifications\\LaunchBlueprint::class, ['alert', 'email'])"); + }); + + it("keeps 2.x's drivers enabled by default", () => { + // As fof/follow-users' 2.x extend.php has it. + const code = upgrade( + 'extend.php', + extendPhp(` ->type(Notifications\\NewFollowerBlueprint::class, ['alert']) + ->type(Notifications\\NewDiscussionBlueprint::class, ['alert', 'email'])`) + ); + + expect(code).toContain("->type(Notifications\\NewFollowerBlueprint::class, ['alert'])"); + expect(code).toContain("->type(Notifications\\NewDiscussionBlueprint::class, ['alert', 'email'])"); + }); + }); + + describe('notification blueprints', () => { + const blueprint = (uses: string, implementsList: string) => ` { + const code = upgrade( + 'src/Notifications/LaunchBlueprint.php', + blueprint('use Flarum\\Notification\\Blueprint\\BlueprintInterface;\nuse Flarum\\User\\User;\n', 'BlueprintInterface') + ); + + expect(code).toContain(`use Flarum\\Notification\\AlertableInterface; +use Flarum\\Notification\\Blueprint\\BlueprintInterface; +use Flarum\\User\\User;`); + expect(code).toContain('class LaunchBlueprint implements BlueprintInterface, AlertableInterface'); + }); + + it('that already do are left as they were', () => { + // As fof/follow-users' 2.x blueprints have it. + const code = blueprint( + 'use Flarum\\Notification\\AlertableInterface;\nuse Flarum\\Notification\\Blueprint\\BlueprintInterface;\nuse Flarum\\Notification\\MailableInterface;\n', + 'BlueprintInterface, MailableInterface, AlertableInterface' + ); + const upgraded = upgrade('src/Notifications/LaunchBlueprint.php', code); + + expect(upgraded.match(/AlertableInterface/g)).toHaveLength(2); + }); + + it('get the import even when they have no other', () => { + const code = upgrade('src/Notifications/LaunchBlueprint.php', blueprint('', '\\Flarum\\Notification\\Blueprint\\BlueprintInterface')); + + expect(code).toContain('use Flarum\\Notification\\AlertableInterface;'); + expect(code).toContain('implements \\Flarum\\Notification\\Blueprint\\BlueprintInterface, AlertableInterface'); + }); + }); +}); diff --git a/test/steps/upgrade/phpunit.test.ts b/test/steps/upgrade/phpunit.test.ts index 7ba1067d..cafbea8e 100644 --- a/test/steps/upgrade/phpunit.test.ts +++ b/test/steps/upgrade/phpunit.test.ts @@ -26,6 +26,25 @@ ${body} `; describe('the PHPUnit step', () => { + it('puts the imports it adds where they sort among those already there', () => { + const code = upgrade( + testClass(` /** + * @test + */ + public function it_works() + { + $this->assertTrue(true); + }`).replace( + 'use Flarum\\Testing\\integration\\TestCase;\n', + 'use Flarum\\Testing\\integration\\TestCase;\nuse Psr\\Http\\Message\\ResponseInterface;\n' + ) + ); + + expect(code).toContain(`use Flarum\\Testing\\integration\\TestCase; +use PHPUnit\\Framework\\Attributes\\Test; +use Psr\\Http\\Message\\ResponseInterface;`); + }); + describe('annotations', () => { it('turns @test and @dataProvider into attributes, and makes the provider static', () => { expect( diff --git a/test/steps/upgrade/typescript.test.ts b/test/steps/upgrade/typescript.test.ts index 2e4e5ad1..93cbe835 100644 --- a/test/steps/upgrade/typescript.test.ts +++ b/test/steps/upgrade/typescript.test.ts @@ -21,8 +21,23 @@ async function upgrade(step: Step, file: string, code: string): Promise return result.updated as string; } +// Read back as TypeScript reads it, comments and all. const upgradeJson = async (step: Step, file: string, content: Record) => - JSON.parse(await upgrade(step, file, JSON.stringify(content, null, 2))); + step.advancedContent(file, await upgrade(step, file, JSON.stringify(content, null, 2))) as Record; + +// What the step puts first in compilerOptions, at the given indentation. +const typesOption = (indent: string) => + `${indent}// Only load the type packages the code imports. Others, such as the\n${indent}// @types/node webpack brings along, use syntax TypeScript 4.9 can't read.\n${indent}"types": [],\n`; + +/** An extension with the given js/package.json, for what the step reads from disk. */ +function extensionWith(pkg: Record): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'typescript-')); + + fs.mkdirSync(path.join(root, 'js')); + fs.writeFileSync(path.join(root, 'js/package.json'), JSON.stringify(pkg)); + + return root; +} const scaffolded = (file: string) => fs.readFileSync(path.resolve(__dirname, '../../../boilerplate/skeleton/extension', file), 'utf8'); @@ -100,6 +115,7 @@ describe('TypeScript, which Flarum 2.0 builds with at 4.9', () => { }); expect(tsconfig.compilerOptions).toEqual({ + types: [], lib: ['DOM'], declarationDir: './dist-typings', baseUrl: '.', @@ -115,10 +131,10 @@ describe('TypeScript, which Flarum 2.0 builds with at 4.9', () => { compilerOptions: { target: 'ESNext', moduleResolution: 'Node', module: 'ES2020', jsx: 'preserve' }, }); - expect(tsconfig.compilerOptions).toEqual({ target: 'ESNext', moduleResolution: 'Node', module: 'ES2020', jsx: 'preserve' }); + expect(tsconfig.compilerOptions).toEqual({ types: [], target: 'ESNext', moduleResolution: 'Node', module: 'ES2020', jsx: 'preserve' }); }); - it('leaves a tsconfig TypeScript 4.9 can read exactly as it was', async () => { + it('leaves a tsconfig TypeScript 4.9 can read as it was, but for the type packages it loads', async () => { const step = newStep(); const code = `{ // Use Flarum's shared TypeScript settings. @@ -131,7 +147,9 @@ describe('TypeScript, which Flarum 2.0 builds with at 4.9', () => { } `; - expect(await upgrade(step, 'js/tsconfig.json', code)).toBe(code); + expect(await upgrade(step, 'js/tsconfig.json', code)).toBe( + code.replace(' "compilerOptions": {\n', ` "compilerOptions": {\n${typesOption(' ')}`) + ); expect(step.pauseMessage()).not.toContain('tsconfig.json'); }); @@ -149,17 +167,72 @@ describe('TypeScript, which Flarum 2.0 builds with at 4.9', () => { } `; + expect(await upgrade(newStep(), 'js/tsconfig.json', code)).toBe( + code.replace(' "compilerOptions": {\n', ` "compilerOptions": {\n${typesOption(' ')}`) + ); + }); + + it('keeps the type packages a tsconfig already names', async () => { + const code = `{\n "compilerOptions": {\n "types": ["mithril"]\n }\n}\n`; + expect(await upgrade(newStep(), 'js/tsconfig.json', code)).toBe(code); }); + it('leaves listing the type packages whose globals the code may use to do', async () => { + const step = new Dependencies( + {} as any, + extensionWith({ devDependencies: { '@types/jquery': '^3.5.0', '@types/jest': '^29.0.0' } }), + 1, + 1, + false + ); + + await upgrade(step, 'js/tsconfig.json', `{\n "compilerOptions": {}\n}\n`); + + expect((step as any).followUps).toEqual([ + { + file: 'js/tsconfig.json', + line: null, + text: 'It only loads the type packages the code imports now. If the code uses the globals of @types/jquery, list them in compilerOptions.types.', + }, + ]); + }); + + it("loads Jest's globals in a test config, which would no longer get them from tsconfig.json", async () => { + const step = new Dependencies({} as any, extensionWith({ devDependencies: { '@flarum/jest-config': '^2.0.0' } }), 1, 1, false); + const code = `{\n "extends": "./tsconfig.json",\n "compilerOptions": {\n "strict": false\n }\n}\n`; + + expect(await upgrade(step, 'js/tsconfig.test.json', code)).toBe( + `{\n "extends": "./tsconfig.json",\n "compilerOptions": {\n "types": ["jest"],\n "strict": false\n }\n}\n` + ); + }); + + it('leaves a test config alone without Jest', async () => { + const code = `{\n "compilerOptions": {\n "strict": false\n }\n}\n`; + + expect(await upgrade(newStep(), 'js/tsconfig.test.json', code)).toBe(code); + }); + + it('is what a new extension is scaffolded with', async () => { + const tsconfig = scaffolded('js/tsconfig.json'); + const test = scaffolded('js/tsconfig.test.json'); + + expect(tsconfig).toContain(typesOption(' ')); + expect(test).toContain('"types": ["jest"]'); + }); + it('flags a list of configs to extend, which TypeScript 4.9 cannot read', async () => { const step = newStep(); const tsconfig = await upgradeJson(step, 'js/tsconfig.json', { extends: ['flarum-tsconfig', './tsconfig.paths.json'] }); expect(tsconfig.extends).toEqual(['flarum-tsconfig', './tsconfig.paths.json']); - expect(step.pauseMessage()).toContain( - 'js/tsconfig.json extends a list of configs, which TypeScript 4.9 cannot read. Extend one, and move the rest into it.' - ); + expect((step as any).followUps).toEqual([ + { + file: 'js/tsconfig.json', + line: null, + text: 'It extends a list of configs, which TypeScript 4.9 cannot read. Extend one, and move the rest into it.', + }, + ]); }); });