feat: add svelte/no-dom-manipulating rule (#302)

* feat: add `svelte/no-dom-manipulating` rule

* Create thick-boxes-warn.md

* chore: update doc

.changeset/thick-boxes-warn.md

@@ -0,0 +1,5 @@
+---
+"eslint-plugin-svelte": minor
+---
+
+feat: add `svelte/no-dom-manipulating` rule

.stylelintignore

@@ -14,3 +14,4 @@ LICENSE
 *.md
 /docs-svelte-kit/
 /coverage
+/build

README.md

@@ -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: |

docs/rules.md

@@ -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: |

docs/rules/no-dom-manipulating.md

@@ -0,0 +1,109 @@
+---
+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 foo, bar, show
+
+  /* ✓ GOOD */
+  const toggle = () => (show = !show)
+
+  /* ✗ BAD */
+  const remove = () => foo.remove()
+  const update = () => (bar.textContent = "Update!")
+</script>
+
+{#if show}
+  <div bind:this={foo}>Foo</div>
+{/if}
+<div bind:this={bar}>
+  {#if show}
+    Bar
+  {/if}
+</div>
+
+<button on:click={() => toggle()}>Click Me (Good)</button>
+<button on:click={() => remove()}>Click Me (Bad)</button>
+<button on:click={() => update()}>Click Me (Bad)</button>
+```
+
+</ESLintCodeBlock>
+
+This rule only tracks and checks variables given with `bind:this={}`. In other words, it doesn't track things like function arguments given to `transition:` directives. These functions have been well tested and are often used more carefully.
+
+<ESLintCodeBlock>
+
+<!--eslint-skip-->
+
+```svelte
+<script>
+  /* eslint svelte/no-dom-manipulating: "error" */
+  let visible = false
+
+  function typewriter(node, { speed = 1 }) {
+    const valid =
+      node.childNodes.length === 1 &&
+      node.childNodes[0].nodeType === Node.TEXT_NODE
+
+    if (!valid) {
+      throw new Error(
+        `This transition only works on elements with a single text node child`,
+      )
+    }
+
+    const text = node.textContent
+    const duration = text.length / (speed * 0.01)
+
+    return {
+      duration,
+      tick: (t) => {
+        const i = Math.trunc(text.length * t)
+        node.textContent = text.slice(0, i) // It does not report.
+      },
+    }
+  }
+</script>
+
+<label>
+  <input type="checkbox" bind:checked={visible} />
+  visible
+</label>
+
+{#if visible}
+  <p transition:typewriter>The quick brown fox jumps over the lazy dog</p>
+{/if}
+```
+
+</ESLintCodeBlock>
+
+See also <https://svelte.jp/examples/custom-js-transitions>.
+
+## :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)

src/rules/no-dom-manipulating.ts

@@ -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")
+      )
+    }
+  },
+})

src/utils/ast-utils.ts

@@ -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(".")
 }
 
 /**

src/utils/rules.ts

@@ -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"
@@ -59,6 +60,7 @@ export const rules = [
   mustacheSpacing,
   noAtDebugTags,
   noAtHtmlTags,
+  noDomManipulating,
   noDupeElseIfBlocks,
   noDupeStyleProperties,
   noDynamicSlotName,

tests/fixtures/rules/no-dom-manipulating/invalid/chain01-errors.yaml

@@ -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

tests/fixtures/rules/no-dom-manipulating/invalid/chain01-input.svelte

@@ -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>

tests/fixtures/rules/no-dom-manipulating/invalid/remove-text01-errors.yaml

@@ -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

tests/fixtures/rules/no-dom-manipulating/invalid/remove-text01-input.svelte

@@ -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>

tests/fixtures/rules/no-dom-manipulating/invalid/remove01-errors.yaml

@@ -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

tests/fixtures/rules/no-dom-manipulating/invalid/remove01-input.svelte

@@ -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>