Skip to content

New ESLint rule: disallow implicit $root schema context #20096

@niegowski

Description

@niegowski

Motivation

After the audit in #20026, most '$root' misuses fall into two patterns that are easy to introduce again:

  1. A feature hardcodes '$root' as a schema context or element name — silently wrong when config.root.modelElement is customised.
  2. A feature calls editor.data.parse( html ) / editor.data.toModel( view ) without an explicit context — picks up the API default, same failure mode.

Both are trivial code review misses. A lint rule can flag them at the source and force the author to either pass the right context or explicitly opt out with a justified eslint-disable comment.

Proposed rule

A single rule with two checks, configurable independently.

Check 1 — no-literal-dollar-root

Disallow the string literals '$root' / "$root" in source files, except in an allowlisted set of packages.

Default allowlist (locations where $root is the generic schema element being defined, not referenced):

  • packages/ckeditor5-engine/**
  • packages/ckeditor5-core/**

Suggested auto-fixes / companion detections:

  • x.is( 'element', '$root' )x.is( 'rootElement' ) — name-agnostic, works for any root. Autofixable.
  • x.name === '$root' / x.name !== '$root'x.is( 'rootElement' ) / !x.is( 'rootElement' ). Autofixable.

Allowed explicitly (do not flag):

  • String literals that are not exactly '$root''$documentFragment', '$clipboardHolder', '$comment', etc. are different schema contexts.
  • Any occurrence inside a comment, JSDoc, or string-template that is not evaluated as a TS string literal node.

Check 2 — require-explicit-data-context

Disallow calling data.parse / data.toModel / upcastDispatcher.convert without an explicit context argument.

Trigger when:

  • The call expression matches <anything>.data.parse( arg ) or <anything>.data.toModel( arg ) with a single argument.
  • Rule fires regardless of the receiver (works for editor.data.parse, this.data.parse, destructured const { data } = editor; data.parse( … ), etc.).

Fix suggestion (non-automatic): "Pass an explicit schema context, or add // eslint-disable-next-line … -- <reason> if you have verified it doesn't matter."

Configuration

{
    "rules": {
        "ckeditor5-rules/no-literal-dollar-root": [ "error", {
            "allowedPackages": [
                "ckeditor5-engine",
                "ckeditor5-core"
            ],
            "allowedCalls": [
                // method names whose string literals are not schema contexts
                "is"
            ]
        } ],
        "ckeditor5-rules/require-explicit-data-context": [ "error", {
            "methods": [ "parse", "toModel" ]
        } ]
    }
}

Opt-out contract

Any eslint-disable-next-line for either rule must include a trailing comment explaining the reason — e.g.:

// eslint-disable-next-line ckeditor5-rules/no-literal-dollar-root -- scratch parse, not applied to the document
const fragment = editor.data.parse( html, '$documentFragment' );

The rule itself can check that the disable comment has a non-empty trailing justification (same pattern as eslint-comments/require-description).

Open ideas

  • allowIn: '$root' detection — a third, optional check that flags schema-definition objects where allowIn contains '$root' but not '$container' / '$block' on elements outside engine. Encourages the allowWhere: '$container' pattern we adopted for GHS htmlHgroup (see audit).
  • schema.extend( '$root', … ) — warn outside engine/core. Every feature extension of $root should go through the documented registerRootAttribute API (or the OSS decision on iterating config.get( 'roots' )).
  • document.createRoot( '$root', … ) / writer.addRoot( _, '$root' ) — same literal check; already covered by Check 1 but maybe worth a dedicated message: "use rootConfig.modelElement or the actual configured element name".
  • Bundled rationale catalogue — ship a short markdown doc alongside the rule that lists the valid reasons for a disable (graveyard root, scratch fragment, dev-utils, known single-root feature, etc.) so reviewers can refer to a shared vocabulary.
  • Retrofit with baseline — the initial rollout should use a .eslint-root-baseline.json or similar that pre-records the currently known violations; the rule only fails on new violations. Prevents a giant migration commit while stopping regressions. The baseline gradually empties as items from Make sure all data parsing or schema related checks use proper/actual root #20026 / commercial#9792 get fixed.
  • Codemod companion — since the .is( 'element', '$root' ).is( 'rootElement' ) swap is mechanical, ship a @ckeditor/ckeditor5-dev-tools codemod to do the bulk conversion when the rule lands.

Definition of Done

  • Rule no-literal-dollar-root implemented with the allowlist config and autofix for .is( 'element', '$root' ) / .name === '$root'.
  • Rule require-explicit-data-context implemented with the method allowlist.
  • Unit tests cover every pattern catalogued in Make sure all data parsing or schema related checks use proper/actual root #20026 and commercial#9792.
  • Baseline file generated from the current repo state so the rule can be enabled without a migration PR.
  • Disable-comment justification requirement enforced.
  • Rule documented in the ESLint-rules README, including the valid-reasons catalogue for disables.
  • Rule enabled in root ESLint config for both ckeditor5 and ckeditor5-commercial repos.

Related

Metadata

Metadata

Assignees

Labels

squad:coreIssue to be handled by the Core team.

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions