Skip to content

Commit

Permalink
feat: add svelte/no-dom-manipulating rule
Browse files Browse the repository at this point in the history
  • Loading branch information
ota-meshi committed Nov 9, 2022
1 parent 1349cf7 commit caa1513
Show file tree
Hide file tree
Showing 30 changed files with 615 additions and 11 deletions.
1 change: 1 addition & 0 deletions .stylelintignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ LICENSE
*.md
/docs-svelte-kit/
/coverage
/build
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ These rules relate to possible syntax or logic errors in Svelte code:

| Rule ID | Description | |
|:--------|:------------|:---|
| [svelte/no-dom-manipulating](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-dom-manipulating/) | disallow DOM manipulating | |
| [svelte/no-dupe-else-if-blocks](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-dupe-else-if-blocks/) | disallow duplicate conditions in `{#if}` / `{:else if}` chains | :star: |
| [svelte/no-dupe-style-properties](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-dupe-style-properties/) | disallow duplicate style properties | :star: |
| [svelte/no-dynamic-slot-name](https://ota-meshi.github.io/eslint-plugin-svelte/rules/no-dynamic-slot-name/) | disallow dynamic slot name | :star::wrench: |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ These rules relate to possible syntax or logic errors in Svelte code:

| Rule ID | Description | |
|:--------|:------------|:---|
| [svelte/no-dom-manipulating](./rules/no-dom-manipulating.md) | disallow DOM manipulating | |
| [svelte/no-dupe-else-if-blocks](./rules/no-dupe-else-if-blocks.md) | disallow duplicate conditions in `{#if}` / `{:else if}` chains | :star: |
| [svelte/no-dupe-style-properties](./rules/no-dupe-style-properties.md) | disallow duplicate style properties | :star: |
| [svelte/no-dynamic-slot-name](./rules/no-dynamic-slot-name.md) | disallow dynamic slot name | :star::wrench: |
Expand Down
54 changes: 54 additions & 0 deletions docs/rules/no-dom-manipulating.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
---
pageClass: "rule-details"
sidebarDepth: 0
title: "svelte/no-dom-manipulating"
description: "disallow DOM manipulating"
---

# svelte/no-dom-manipulating

> disallow DOM manipulating
- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>

## :book: Rule Details

In general, DOM manipulating should delegate to Svelte runtime. If you manipulate the DOM directly, the Svelte runtime may confuse because there is a difference between the actual DOM and the Svelte runtime's expected DOM.
Therefore this rule reports where you use DOM manipulating function.
We don't recommend but If you intentionally manipulate the DOM, simply you can ignore this ESLint report.

<ESLintCodeBlock>

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/no-dom-manipulating: "error" */
let div
let show
/* ✓ GOOD */
const toggle = () => (show = !show)
/* ✗ BAD */
const remove = () => div.remove()
</script>
{#if show}
<div bind:this={div}>div</div>
{/if}
<button on:click={() => toggle()}>Click Me (Good)</button>
<button on:click={() => remove()}>Click Me (Bad)</button>
```

</ESLintCodeBlock>

## :wrench: Options

Nothing.

## :mag: Implementation

- [Rule source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/src/rules/no-dom-manipulating.ts)
- [Test source](https://github.com/ota-meshi/eslint-plugin-svelte/blob/main/tests/src/rules/no-dom-manipulating.ts)
131 changes: 131 additions & 0 deletions src/rules/no-dom-manipulating.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import type { AST } from "svelte-eslint-parser"
import type { TSESTree } from "@typescript-eslint/types"
import { createRule } from "../utils"
import { findVariable, getNodeName } from "../utils/ast-utils"
import type { Variable } from "@typescript-eslint/scope-manager"
import { getPropertyName } from "eslint-utils"

const DOM_MANIPULATING_METHODS = new Set([
"appendChild", // https://developer.mozilla.org/en-US/docs/Web/API/Node/appendChild
"insertBefore", // https://developer.mozilla.org/en-US/docs/Web/API/Node/insertBefore
"normalize", // https://developer.mozilla.org/en-US/docs/Web/API/Node/normalize
"removeChild", // https://developer.mozilla.org/en-US/docs/Web/API/Node/removeChild
"replaceChild", // https://developer.mozilla.org/en-US/docs/Web/API/Node/replaceChild
"after", // https://developer.mozilla.org/en-US/docs/Web/API/Element/after
"append", // https://developer.mozilla.org/en-US/docs/Web/API/Element/append
"before", // https://developer.mozilla.org/en-US/docs/Web/API/Element/before
"insertAdjacentElement", // https://developer.mozilla.org/en-US/docs/Web/API/Element/insertAdjacentElement
"insertAdjacentHTML", // https://developer.mozilla.org/en-US/docs/Web/API/Element/insertAdjacentHTML
"insertAdjacentText", // https://developer.mozilla.org/en-US/docs/Web/API/Element/insertAdjacentText
"prepend", // https://developer.mozilla.org/en-US/docs/Web/API/Element/prepend
"remove", // https://developer.mozilla.org/en-US/docs/Web/API/Element/remove
"replaceChildren", // https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceChildren
"replaceWith", // https://developer.mozilla.org/en-US/docs/Web/API/Element/replaceWith
])
const DOM_MANIPULATING_PROPERTIES = new Set([
"textContent", // https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent
"innerHTML", // https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML
"outerHTML", // https://developer.mozilla.org/en-US/docs/Web/API/Element/outerHTML
"innerText", // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/innerText
"outerText", // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/outerText
])

export default createRule("no-dom-manipulating", {
meta: {
docs: {
description: "disallow DOM manipulating",
category: "Possible Errors",
recommended: false,
},
schema: [],
messages: {
disallowManipulateDOM:
"Don't manipulate the DOM directly. The Svelte runtime can get confused if there is a difference between the actual DOM and the DOM expected by the Svelte runtime.",
},
type: "problem",
},
create(context) {
const domVariables = new Set<Variable>()

/**
* Verify DOM variable identifier node
*/
function verifyIdentifier(
node: TSESTree.Identifier | TSESTree.JSXIdentifier,
) {
const member = node.parent
if (member?.type !== "MemberExpression" || member.object !== node) {
return
}
const name = getPropertyName(member)
if (!name) {
return
}
let target: TSESTree.Expression = member
let parent = target.parent
while (parent?.type === "ChainExpression") {
target = parent
parent = parent.parent
}
if (!parent) {
return
}
if (parent.type === "CallExpression") {
if (parent.callee !== target || !DOM_MANIPULATING_METHODS.has(name)) {
return
}
} else if (parent.type === "AssignmentExpression") {
if (parent.left !== target || !DOM_MANIPULATING_PROPERTIES.has(name)) {
return
}
}
context.report({
node: member,
messageId: "disallowManipulateDOM",
})
}

return {
"SvelteDirective[kind='Binding']"(node: AST.SvelteBindingDirective) {
if (
node.key.name.name !== "this" ||
!node.expression ||
node.expression.type !== "Identifier"
) {
// not bind:this={id}
return
}
const element = node.parent.parent
if (element.type !== "SvelteElement" || !isHTMLElement(element)) {
// not HTML element
return
}
const variable = findVariable(context, node.expression)
if (
!variable ||
(variable.scope.type !== "module" && variable.scope.type !== "global")
) {
return
}
domVariables.add(variable)
},
"Program:exit"() {
for (const variable of domVariables) {
for (const reference of variable.references) {
verifyIdentifier(reference.identifier)
}
}
},
}

/**
* Checks whether the given node is a HTML element or not.
*/
function isHTMLElement(node: AST.SvelteElement) {
return (
node.kind === "html" ||
(node.kind === "special" && getNodeName(node) === "svelte:element")
)
}
},
})
14 changes: 6 additions & 8 deletions src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,19 +518,17 @@ function getAttributeValueRangeTokens(
* Returns name of SvelteElement
*/
export function getNodeName(node: SvAST.SvelteElement): string {
if ("name" in node.name) {
if (node.name.type === "Identifier" || node.name.type === "SvelteName") {
return node.name.name
}
let object = ""
const memberPath = [node.name.property.name]
let currentObject = node.name.object
while ("object" in currentObject) {
object = `${currentObject.property.name}.${object}`
while (currentObject.type === "SvelteMemberExpressionName") {
memberPath.unshift(currentObject.property.name)
currentObject = currentObject.object
}
if ("name" in currentObject) {
object = `${currentObject.name}.${object}`
}
return object + node.name.property.name
memberPath.unshift(currentObject.name)
return memberPath.join(".")
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import maxAttributesPerLine from "../rules/max-attributes-per-line"
import mustacheSpacing from "../rules/mustache-spacing"
import noAtDebugTags from "../rules/no-at-debug-tags"
import noAtHtmlTags from "../rules/no-at-html-tags"
import noDomManipulating from "../rules/no-dom-manipulating"
import noDupeElseIfBlocks from "../rules/no-dupe-else-if-blocks"
import noDupeStyleProperties from "../rules/no-dupe-style-properties"
import noDynamicSlotName from "../rules/no-dynamic-slot-name"
Expand Down Expand Up @@ -59,6 +60,7 @@ export const rules = [
mustacheSpacing,
noAtDebugTags,
noAtHtmlTags,
noDomManipulating,
noDupeElseIfBlocks,
noDupeStyleProperties,
noDynamicSlotName,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 5
column: 5
suggestions: null
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 9
column: 7
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script>
let foo
const remove1 = () => {
foo?.remove()
}
const remove2 = () => {
// eslint-disable-next-line no-unsafe-optional-chaining -- ignore
;(foo?.remove)()
}
</script>
<p bind:this={foo}>div</p>
<button on:click={() => remove1()}>Click Me</button>
<button on:click={() => remove2()}>Click Me</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 9
column: 25
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<script>
let div
let show
// ✓ GOOD
const toggle = () => (show = !show)
// ✗ BAD
const update = () => (div.textContent = "foo")
</script>

<div bind:this={div}>
{#if show}
div
{/if}
</div>

<button on:click={() => toggle()}>Click Me (Good)</button>
<button on:click={() => update()}>Click Me (Bad)</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 9
column: 24
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
let div
let show
// ✓ GOOD
const toggle = () => (show = !show)
// ✗ BAD
const remove = () => div.remove()
</script>

{#if show}
<div bind:this={div}>div</div>
{/if}

<button on:click={() => toggle()}>Click Me (Good)</button>
<button on:click={() => remove()}>Click Me (Bad)</button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 7
column: 5
suggestions: null
- message:
Don't manipulate the DOM directly. The Svelte runtime can get confused
if there is a difference between the actual DOM and the DOM expected by the
Svelte runtime.
line: 8
column: 5
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<script>
export let tag = "div"
let foo
let bar
const remove = () => {
foo.remove()
bar.remove()
}
</script>

<p bind:this={foo}>div</p>
<svelte:element this={tag} bind:this={bar}>div</svelte:element>

<button on:click={() => remove()}>Click Me</button>
Loading

0 comments on commit caa1513

Please sign in to comment.