diff --git a/javascript/packages/linter/docs/rules/README.md b/javascript/packages/linter/docs/rules/README.md index 88c44edf4..a72f548b9 100644 --- a/javascript/packages/linter/docs/rules/README.md +++ b/javascript/packages/linter/docs/rules/README.md @@ -59,6 +59,7 @@ This page contains documentation for all Herb Linter rules. - [`html-tag-name-lowercase`](./html-tag-name-lowercase.md) - Enforces lowercase tag names in HTML - [`parser-no-errors`](./parser-no-errors.md) - Disallow parser errors in HTML+ERB documents - [`svg-tag-name-capitalization`](./svg-tag-name-capitalization.md) - Enforces proper camelCase capitalization for SVG elements +- [`turbo-permanent-require-id`](./turbo-permanent-require-id.md) - Require `id` attribute on elements with `data-turbo-permanent` ## Contributing diff --git a/javascript/packages/linter/docs/rules/turbo-permanent-require-id.md b/javascript/packages/linter/docs/rules/turbo-permanent-require-id.md new file mode 100644 index 000000000..04801d4cb --- /dev/null +++ b/javascript/packages/linter/docs/rules/turbo-permanent-require-id.md @@ -0,0 +1,41 @@ +# Linter Rule: Require `id` attribute on elements with `data-turbo-permanent` + +**Rule:** `turbo-permanent-require-id` + +## Description + +Ensure that all HTML elements with the `data-turbo-permanent` attribute also have an `id` attribute. Without an `id`, Turbo can't track the element across page changes and the permanent behavior won't work as expected. + +## Rationale + +Turbo's `data-turbo-permanent` attribute marks an element to be persisted across page navigations. Turbo uses the element's `id` to match it between the current page and the new page. If no `id` is present, Turbo has no way to identify and preserve the element, so the `data-turbo-permanent` attribute has no effect. + +## Examples + +### ✅ Good + +```erb +
+ +
+ + +``` + +### 🚫 Bad + +```erb +
+ +
+ + +``` + +## References + +- [Turbo Handbook: Persisting Elements Across Page Loads](https://turbo.hotwired.dev/handbook/building#persisting-elements-across-page-loads) diff --git a/javascript/packages/linter/src/rules.ts b/javascript/packages/linter/src/rules.ts index 1d430ab59..f9e982a19 100644 --- a/javascript/packages/linter/src/rules.ts +++ b/javascript/packages/linter/src/rules.ts @@ -59,6 +59,7 @@ import { HTMLNoTitleAttributeRule } from "./rules/html-no-title-attribute.js" import { HTMLNoUnderscoresInAttributeNamesRule } from "./rules/html-no-underscores-in-attribute-names.js" import { HTMLRequireClosingTagsRule } from "./rules/html-require-closing-tags.js" import { HTMLTagNameLowercaseRule } from "./rules/html-tag-name-lowercase.js" +import { TurboPermanentRequireIdRule } from "./rules/turbo-permanent-require-id.js" import { SVGTagNameCapitalizationRule } from "./rules/svg-tag-name-capitalization.js" @@ -124,6 +125,7 @@ export const rules: RuleClass[] = [ HTMLNoUnderscoresInAttributeNamesRule, HTMLRequireClosingTagsRule, HTMLTagNameLowercaseRule, + TurboPermanentRequireIdRule, SVGTagNameCapitalizationRule, diff --git a/javascript/packages/linter/src/rules/turbo-permanent-require-id.ts b/javascript/packages/linter/src/rules/turbo-permanent-require-id.ts new file mode 100644 index 000000000..dba4fd264 --- /dev/null +++ b/javascript/packages/linter/src/rules/turbo-permanent-require-id.ts @@ -0,0 +1,48 @@ +import { BaseRuleVisitor, getAttribute } from "./rule-utils.js" + +import { ParserRule } from "../types.js" +import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js" +import type { HTMLOpenTagNode, ParseResult } from "@herb-tools/core" + +class TurboPermanentRequireIdVisitor extends BaseRuleVisitor { + visitHTMLOpenTagNode(node: HTMLOpenTagNode): void { + this.checkTurboPermanent(node) + super.visitHTMLOpenTagNode(node) + } + + private checkTurboPermanent(node: HTMLOpenTagNode): void { + const turboPermanentAttribute = getAttribute(node, "data-turbo-permanent") + + if (!turboPermanentAttribute) { + return + } + + const idAttribute = getAttribute(node, "id") + + if (!idAttribute) { + this.addOffense( + "Elements with `data-turbo-permanent` must have an `id` attribute. Without an `id`, Turbo can't track the element across page changes and the permanent behavior won't work as expected.", + turboPermanentAttribute.location, + ) + } + } +} + +export class TurboPermanentRequireIdRule extends ParserRule { + name = "turbo-permanent-require-id" + + get defaultConfig(): FullRuleConfig { + return { + enabled: true, + severity: "error" + } + } + + check(result: ParseResult, context?: Partial): UnboundLintOffense[] { + const visitor = new TurboPermanentRequireIdVisitor(this.name, context) + + visitor.visit(result.value) + + return visitor.offenses + } +} diff --git a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap index 04f27753c..7c716f53b 100644 --- a/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap +++ b/javascript/packages/linter/test/__snapshots__/cli.test.ts.snap @@ -787,7 +787,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for bad file 1`] "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 54, + "ruleCount": 55, "totalErrors": 2, "totalHints": 0, "totalIgnored": 0, @@ -808,7 +808,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for clean file 1` "summary": { "filesChecked": 1, "filesWithOffenses": 0, - "ruleCount": 54, + "ruleCount": 55, "totalErrors": 0, "totalHints": 0, "totalIgnored": 0, @@ -881,7 +881,7 @@ exports[`CLI Output Formatting > formats JSON output correctly for file with err "summary": { "filesChecked": 1, "filesWithOffenses": 1, - "ruleCount": 54, + "ruleCount": 55, "totalErrors": 3, "totalHints": 0, "totalIgnored": 0, diff --git a/javascript/packages/linter/test/rules/turbo-permanent-require-id-rule.test.ts b/javascript/packages/linter/test/rules/turbo-permanent-require-id-rule.test.ts new file mode 100644 index 000000000..09c17c24f --- /dev/null +++ b/javascript/packages/linter/test/rules/turbo-permanent-require-id-rule.test.ts @@ -0,0 +1,58 @@ +import { describe, test } from "vitest" +import { TurboPermanentRequireIdRule } from "../../src/rules/turbo-permanent-require-id.js" +import { createLinterTest } from "../helpers/linter-test-helper.js" + +const { expectNoOffenses, expectError, assertOffenses } = createLinterTest(TurboPermanentRequireIdRule) + +const MESSAGE = "Elements with `data-turbo-permanent` must have an `id` attribute. Without an `id`, Turbo can't track the element across page changes and the permanent behavior won't work as expected." + +describe("turbo-permanent-require-id", () => { + test("passes for element with data-turbo-permanent and id", () => { + expectNoOffenses('
Flash
') + }) + + test("fails for element with data-turbo-permanent but no id", () => { + expectError(MESSAGE) + + assertOffenses('
Flash
') + }) + + test("passes for element without data-turbo-permanent", () => { + expectNoOffenses('
Content
') + }) + + test("passes for element with only id", () => { + expectNoOffenses('
Content
') + }) + + test("fails for self-closing element with data-turbo-permanent but no id", () => { + expectError(MESSAGE) + + assertOffenses('') + }) + + test("passes for self-closing element with data-turbo-permanent and id", () => { + expectNoOffenses('') + }) + + test("fails for multiple elements with data-turbo-permanent but no id", () => { + expectError(MESSAGE) + expectError(MESSAGE) + + assertOffenses('
Flash
\nNotice') + }) + + test("passes for element with data-turbo-permanent and ERB id", () => { + expectNoOffenses('
Content
') + }) + + test("passes for element with data-turbo-permanent value and id", () => { + expectNoOffenses('
Cart
') + }) + + test("fails for element with other data attributes but no id", () => { + expectError(MESSAGE) + + assertOffenses('
Flash
') + }) +})