The_Great_Archive/trilium
1
0
Fork 0
mirror of https://github.com/zadam/trilium.git synced 2026-09-12 19:50:20 +05:00
Code Issues Actions 18 Packages Projects Releases Wiki Activity
e8eeb68b50
trilium/.claude/skills/ckeditor5-reviewing/SKILL.md
Elian Doran e8eeb68b50
test(ckeditor5): run browser mode on Playwright instead of webdriverio 
Vitest 5 handed the webdriverio provider to community maintenance, and no
stable 5.x of `@vitest/browser-webdriverio` was ever published — npm `latest`
is still 4.1.11, with only a 5.0.0-rc.1 from before vitest 5.0.0 shipped.
Pairing that v4 provider with `@vitest/browser` 5 kills the suite at startup:

    TypeError: Cannot read properties of undefined (reading 'project')
        at createBrowserServer (@vitest/browser/dist/index.js:7900:26)

`@vitest/browser-playwright` 5.0.0 is stable and first-party, and Playwright
1.62.1 is already here for the e2e suites, so the browser toolchain costs
nothing new. Only two of the 129 specs touch provider-visible API at all, both
through the provider-agnostic `userEvent`.

Playwright drives the browser over CDP with no separate driver, so the
chromedriver half of the NixOS workaround goes away: the dev shell keeps
`pkgs.chromium` and `CHROME_BIN`, which now reaches the provider as
`launchOptions.executablePath`. CI installs the browser in a step of its own
rather than inside the test step, whose 15-minute cap exists to catch a browser
session that never starts and should not also have to cover a 190 MB download.

Two specs depended on webdriverio behaviour and are fixed rather than skipped:

- The token-cost assertion read `1.234` where it wanted `1,234`.
  `toLocaleString()` takes the browser's locale, and Playwright inherits the
  host's where the old Chrome defaulted to en-US. Pinning `contextOptions.locale`
  keeps the suite from depending on the developer's machine.
- The format painter's drag-selection selected nothing. Playwright's
  `dragAndDrop` turns on drag interception, so the press reaches the page as an
  HTML5 drag intent instead of selecting text. Driving the press, move and
  release over CDP restores what the test is actually for: proving a *native*
  pointer interaction has updated the model selection by the time the `mouseup`
  listener runs.

Vitest 5 also moved failure screenshots from `.vitest-attachments` to
`.vitest`, which needs ignoring.

Verified: 127 files / 1626 tests green, coverage 99.91/99.54/100/99.98 against
the 99.5 gate. The Nix path is unexercised — `executablePath` is ordinary
Playwright, but it was not run from a dev shell.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 23:19:52 +02:00

9.7 KiB
Raw Blame History

name description
ckeditor5-reviewing Review or audit CKEditor 5 plugin code in the Trilium (TriliumNext Notes) monorepo, or a PR/diff touching packages/ckeditor5 (including its in-tree plugins under src/plugins/). Use when checking a Trilium CKEditor 5 plugin for correctness and idiom: schema / conversion / command / UI / widget code, CKEditor-specific defects (asymmetric upcast/downcast, unconsumed upcast elements, missing inline-widget position mapping, command refresh/isEnabled bugs, memory leaks, t() gaps, editing/UI split violations), and Trilium integration defects (plugin not registered in plugins.ts, button missing from toolbar.ts, import/file-extension lint failures, wrong augmentation module, wrong DOM assumptions in tests). Pairs with the ckeditor5-plugin-development and ckeditor5-testing skills and delegates their checklists.

Reviewing CKEditor 5 plugins in Trilium

A workflow and defect catalog for reviewing CKEditor 5 plugin code in the Trilium monorepo — a distinct task from writing it. This skill orchestrates the review and hunts both CKEditor-specific and Trilium-integration bugs; it delegates the per-dimension "is this idiomatic?" checklists to the companion skills rather than duplicating them:

  • ckeditor5-plugin-development — its references/review-checklist.md (architecture, schema, conversion, commands, UI, a11y) and references/conventions.md (naming, imports, JSDoc, TS).
  • ckeditor5-testing — its review checklist and patterns for the test side (Vitest, browser-mode vs. @vitest/browser-playwright browser mode, real ClassicEditor.create).

Use those for "does this follow the conventions"; use this skill for how to drive the review and what subtle things tend to be wrong.

Scope & sources

This skill reviews CKEditor 5 plugin code in the Trilium monorepo (scope @triliumnext/). The CKEditor 5 library itself is the external ckeditor5 dependency, 48 or later — its docs (ckeditor.com/docs) and source (github.com/ckeditor/ckeditor5) are external references, not the code under review. Nearly all Trilium plugins live inside packages/ckeditor5, under src/plugins/<name>/ — admonition, collapsible, footnotes, keyboard_marker, mermaid and the in-tree plugins alongside them. There are no separate CKEditor plugin packages left. The aggregate assembles them all and is wired into the editor UI from apps/client.

On versions: these skills name major versions only ("48 or later"). Trilium tracks CKEditor 5 closely, so an exact pin written here would be stale within weeks — read the current one from packages/ckeditor5/package.json.

When to use

Reviewing a PR or diff that touches a Trilium CKEditor 5 plugin (packages/ckeditor5/src/plugins/ ), the aggregate itself, or the toolbar config in apps/client; auditing an existing plugin; sanity-checking your own feature before opening a PR. For writing the feature, use ckeditor5-plugin-development; for writing tests, use ckeditor5-testing.

Review workflow

  1. Scope the diff against Trilium's structure. What surface changed — editing (schema/conversion/command), UI (componentFactory/buttons/dropdowns/balloons), a widget, config? Which plugin (packages/ckeditor5/src/plugins/<name>/)? Does it also touch the aggregate's wiring (packages/ckeditor5/src/plugins.ts, src/index.ts) or the toolbar (apps/client/src/widgets/type_widgets/text/toolbar.ts)? This tells you which checklists, defect groups, and integration checks apply.
  2. Read model-first. The model is the source of truth; trace the feature in order schema → conversion → command → UI. Confirm each layer is present and consistent (e.g. a new model element has a schema registration and upcast and downcast and an insertion path).
  3. Check registration & wiring (Trilium-specific). A correct plugin that nobody loads is still broken. Confirm:
    • the plugin is exported and added to the right array in packages/ckeditor5/src/plugins.ts (CORE_PLUGINS / TRILIUM_PLUGINS / EXTERNAL_PLUGINS → builtinPlugins) so it actually loads into the editor classes in packages/ckeditor5/src/index.ts (AttributeEditor[Balloon], ClassicEditor[Decoupled], PopupEditor[Balloon]);
    • any new toolbar component is added to apps/client/src/widgets/type_widgets/text/toolbar.ts, otherwise the button never appears even though the plugin loaded.
  4. Check conventions (Trilium-specific). Imports come from the ckeditor5 aggregate (or a relative path inside packages/ckeditor5) with explicit file extensions; augmentation uses declare module 'ckeditor5' (never @ckeditor/ckeditor5-core), normally at the bottom of the plugin's glue file; any license header matches the sibling files in that plugin folder and the provenance recorded in its README.md. These are enforced by eslint-config-ckeditor5 (require-file-extensions-in-imports, allow-imports-only-from-main-package-entry-point, no-legacy-imports) and stylelint-config-ckeditor5 — a diff that breaks them fails lint.
  5. Run the tests for the affected package. Use Vitest via pnpm --filter @triliumnext/ckeditor5 test (or ...-math); the two run sequentially because each spins up headless Chromium. Both use @vitest/browser-playwright browser mode and gate src/** at 100% coverage. Coverage ≠ correctness: confirm the change itself is tested, not just that lines are hit. A bug fix with no new/changed test is a red flag even when coverage stays green.
  6. Observe behavior. Attach the CKEditor Inspector (model / view / schema / commands), then: round-trip editor.getData() → setData() (does content survive a save?); exercise selection edge cases (collapsed vs. ranged, inside objects/limits, at attribute boundaries); toggle read-only.
  7. Apply the dimension checklists (delegate to the two companion skills).
  8. Hunt defects with references/defect-patterns.md — the high-value, easy-to-miss failure modes (CKEditor-general + Trilium integration).
  9. Verify each finding before reporting it (see below). Reproduce it, or cite the exact line and the rule it breaks. Discard the ones that don't hold up.
  10. Report findings: severity-tagged, file:line, with the concrete fix or question.

Triage / severity

  • Blocker — data loss (content dropped on getData()), crashes/console errors, undo corruption, security (unsanitized HTML/XSS), schema corruption; plugin not registered in plugins.ts (feature silently never loads).
  • Major — incorrect behavior in real selections, accessibility gaps (no keyboard path, missing labels/addKeystrokeInfos), memory leaks, command enabled where the schema disallows it; button missing from toolbar.ts; lint-failing imports (missing file extension / wrong package / legacy @ckeditor/*) that block the build.
  • Minor — convention/naming issues, missing t(), redundant converters, non-idiomatic but working code.
  • Nit — style preferences with no behavioral impact; mark them as optional.

Lead with blockers/majors; don't bury them under nits.

Verify before reporting (avoid false positives)

A noisy review erodes trust. Confirm a finding is real before raising it. Common false positives to not raise:

  • "Missing downcast" when the code uses a two-way helper (conversion.attributeToElement(...) registers both directions) — count two-way helpers, not just for('dataDowncast').
  • "getAttribute('data-x') should be .dataset" inside a converter callback — that's a view element, not DOM; .dataset would silently drop the value (see the plugin-dev conversion.md view-element pitfall).
  • "Boolean attribute should be set to false" — the idiom is to remove it (yields undefined).
  • "Button state not updated" when it's bind()-ed to the command (reactive, not imperative).
  • "Listener never removed" when added via this.listenTo() (auto-cleaned in destroy()).
  • "Plugin not registered" when it's pulled in transitively via another plugin's static get requires() rather than listed directly in plugins.ts — check the dependency chain before flagging.
  • "Import should be from @ckeditor/ckeditor5-*" — Trilium deliberately imports from the ckeditor5 aggregate (with file extensions); the deep @ckeditor/* path is the lint failure, not the fix.

When unsure, phrase it as a question ("Is <mark> intended to round-trip through getData()? I don't see a dataDowncast.") rather than a false assertion.

Reference map

File Use it for
references/defect-patterns.md The bug catalog — CKEditor-general groups (conversion, schema, commands, UI, lifecycle, localization, undo, tests) plus a Trilium integration group. For each: the symptom, how to spot it in a diff, why it's wrong, and the fix. The core of a correctness review.

Provenance & source references

The CKEditor-general defect patterns are distilled from the companion skills and the CKEditor 5 library at 48 or later; docs/… / packages/… paths in those patterns point into the external CKEditor 5 repository (github.com/ckeditor/ckeditor5), not the Trilium code under review. The Trilium-specific paths (packages/ckeditor5/src/plugins/<name>/, packages/ckeditor5/src/{plugins,index}.ts, apps/client/src/widgets/type_widgets/text/toolbar.ts) point into the Trilium monorepo. To refresh, re-check those paths against the current Trilium tree and bump the CKEditor version if the ckeditor5 pin changes.