Skip to content

Commit ebeeb5c

Browse files
authored
Linter: Implement turbo-permanent-require-id rule (#1229)
Resolves #1183
1 parent 3f3cfe6 commit ebeeb5c

File tree

6 files changed

+153
-3
lines changed

6 files changed

+153
-3
lines changed

javascript/packages/linter/docs/rules/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ This page contains documentation for all Herb Linter rules.
5959
- [`html-tag-name-lowercase`](./html-tag-name-lowercase.md) - Enforces lowercase tag names in HTML
6060
- [`parser-no-errors`](./parser-no-errors.md) - Disallow parser errors in HTML+ERB documents
6161
- [`svg-tag-name-capitalization`](./svg-tag-name-capitalization.md) - Enforces proper camelCase capitalization for SVG elements
62+
- [`turbo-permanent-require-id`](./turbo-permanent-require-id.md) - Require `id` attribute on elements with `data-turbo-permanent`
6263

6364
## Contributing
6465

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Linter Rule: Require `id` attribute on elements with `data-turbo-permanent`
2+
3+
**Rule:** `turbo-permanent-require-id`
4+
5+
## Description
6+
7+
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.
8+
9+
## Rationale
10+
11+
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.
12+
13+
## Examples
14+
15+
### ✅ Good
16+
17+
```erb
18+
<div id="player" data-turbo-permanent>
19+
<!-- This element will persist across page navigations -->
20+
</div>
21+
22+
<audio id="background-music" data-turbo-permanent>
23+
<source src="/music.mp3" type="audio/mpeg">
24+
</audio>
25+
```
26+
27+
### 🚫 Bad
28+
29+
```erb
30+
<div data-turbo-permanent>
31+
<!-- Missing id: Turbo can't track this element -->
32+
</div>
33+
34+
<audio data-turbo-permanent>
35+
<source src="/music.mp3" type="audio/mpeg">
36+
</audio>
37+
```
38+
39+
## References
40+
41+
- [Turbo Handbook: Persisting Elements Across Page Loads](https://turbo.hotwired.dev/handbook/building#persisting-elements-across-page-loads)

javascript/packages/linter/src/rules.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import { HTMLNoTitleAttributeRule } from "./rules/html-no-title-attribute.js"
5959
import { HTMLNoUnderscoresInAttributeNamesRule } from "./rules/html-no-underscores-in-attribute-names.js"
6060
import { HTMLRequireClosingTagsRule } from "./rules/html-require-closing-tags.js"
6161
import { HTMLTagNameLowercaseRule } from "./rules/html-tag-name-lowercase.js"
62+
import { TurboPermanentRequireIdRule } from "./rules/turbo-permanent-require-id.js"
6263

6364
import { SVGTagNameCapitalizationRule } from "./rules/svg-tag-name-capitalization.js"
6465

@@ -124,6 +125,7 @@ export const rules: RuleClass[] = [
124125
HTMLNoUnderscoresInAttributeNamesRule,
125126
HTMLRequireClosingTagsRule,
126127
HTMLTagNameLowercaseRule,
128+
TurboPermanentRequireIdRule,
127129

128130
SVGTagNameCapitalizationRule,
129131

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { BaseRuleVisitor, getAttribute } from "./rule-utils.js"
2+
3+
import { ParserRule } from "../types.js"
4+
import type { UnboundLintOffense, LintContext, FullRuleConfig } from "../types.js"
5+
import type { HTMLOpenTagNode, ParseResult } from "@herb-tools/core"
6+
7+
class TurboPermanentRequireIdVisitor extends BaseRuleVisitor {
8+
visitHTMLOpenTagNode(node: HTMLOpenTagNode): void {
9+
this.checkTurboPermanent(node)
10+
super.visitHTMLOpenTagNode(node)
11+
}
12+
13+
private checkTurboPermanent(node: HTMLOpenTagNode): void {
14+
const turboPermanentAttribute = getAttribute(node, "data-turbo-permanent")
15+
16+
if (!turboPermanentAttribute) {
17+
return
18+
}
19+
20+
const idAttribute = getAttribute(node, "id")
21+
22+
if (!idAttribute) {
23+
this.addOffense(
24+
"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.",
25+
turboPermanentAttribute.location,
26+
)
27+
}
28+
}
29+
}
30+
31+
export class TurboPermanentRequireIdRule extends ParserRule {
32+
name = "turbo-permanent-require-id"
33+
34+
get defaultConfig(): FullRuleConfig {
35+
return {
36+
enabled: true,
37+
severity: "error"
38+
}
39+
}
40+
41+
check(result: ParseResult, context?: Partial<LintContext>): UnboundLintOffense[] {
42+
const visitor = new TurboPermanentRequireIdVisitor(this.name, context)
43+
44+
visitor.visit(result.value)
45+
46+
return visitor.offenses
47+
}
48+
}

javascript/packages/linter/test/__snapshots__/cli.test.ts.snap

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { describe, test } from "vitest"
2+
import { TurboPermanentRequireIdRule } from "../../src/rules/turbo-permanent-require-id.js"
3+
import { createLinterTest } from "../helpers/linter-test-helper.js"
4+
5+
const { expectNoOffenses, expectError, assertOffenses } = createLinterTest(TurboPermanentRequireIdRule)
6+
7+
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."
8+
9+
describe("turbo-permanent-require-id", () => {
10+
test("passes for element with data-turbo-permanent and id", () => {
11+
expectNoOffenses('<div id="flash-messages" data-turbo-permanent>Flash</div>')
12+
})
13+
14+
test("fails for element with data-turbo-permanent but no id", () => {
15+
expectError(MESSAGE)
16+
17+
assertOffenses('<div data-turbo-permanent>Flash</div>')
18+
})
19+
20+
test("passes for element without data-turbo-permanent", () => {
21+
expectNoOffenses('<div class="container">Content</div>')
22+
})
23+
24+
test("passes for element with only id", () => {
25+
expectNoOffenses('<div id="my-element">Content</div>')
26+
})
27+
28+
test("fails for self-closing element with data-turbo-permanent but no id", () => {
29+
expectError(MESSAGE)
30+
31+
assertOffenses('<input data-turbo-permanent>')
32+
})
33+
34+
test("passes for self-closing element with data-turbo-permanent and id", () => {
35+
expectNoOffenses('<input id="my-input" data-turbo-permanent>')
36+
})
37+
38+
test("fails for multiple elements with data-turbo-permanent but no id", () => {
39+
expectError(MESSAGE)
40+
expectError(MESSAGE)
41+
42+
assertOffenses('<div data-turbo-permanent>Flash</div>\n<span data-turbo-permanent>Notice</span>')
43+
})
44+
45+
test("passes for element with data-turbo-permanent and ERB id", () => {
46+
expectNoOffenses('<div id="<%= dom_id(record) %>" data-turbo-permanent>Content</div>')
47+
})
48+
49+
test("passes for element with data-turbo-permanent value and id", () => {
50+
expectNoOffenses('<div id="cart" data-turbo-permanent="">Cart</div>')
51+
})
52+
53+
test("fails for element with other data attributes but no id", () => {
54+
expectError(MESSAGE)
55+
56+
assertOffenses('<div class="flash" data-turbo-permanent data-controller="flash">Flash</div>')
57+
})
58+
})

0 commit comments

Comments
 (0)