svelte/no-dom-manipulating

[baseballyama]

Motivation

Many Svelte users face Uncaught (in promise) TypeError: Cannot read property 'removeChild' of null.

sveltejs/svelte#6037

This is because userland code (or library code) manipulates DOM directly.
Then between the actual DOM and Svelte runtime expected DOM is different and such an error occurs.

Some user wants to use a library that includes DOM manipulation, so I think the Svelte runtime also need to update.
But at the same time, always actual DOM and Svelte expected DOM also should be same as much as possible.
Therefore we should not use DOM manipulating functions.

Description

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.

Examples

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

Additional comments

I think Vue also can face a similar situation but I couldn't find a such rule.
(I tried to run with Vue.js at playground and the situation is the same.)
I would like to know why Vue.js doesn't need to care about it.

[ota-meshi]

Thank you for the rule suggestions! That rule sounds good to me.
But I think we should consider what DOM manipulations are reported and not reported.
For example, the transition tutorial manipulates textContent and should be fine. This should be considered whether textContent manipulations should be ignored from reporting, or animation related manipulations should be ignored from reporting.
https://svelte.jp/tutorial/custom-js-transitions

I think the reason eslint-plugin-vue doesn't have that rule is because no one has suggested it. At least there hasn't been a suggestion for that rule since I started maintaining it.

[baseballyama]

Ohh.
So we should report only DOM adding/deleting operations?
Because I think the critical point is changing the DOM structure.

[hallvors]

What if Svelte keeps a reference to a text node and you set textContent on the parent?

[ota-meshi]

I tried it. It seems that updating the textContent can also cause errors.

https://svelte.dev/repl/2b02b103ace347b8aaf2d45311829f4d?version=3.52.0

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

So I think it's reasonable to ignore DOM manipulations by directives (transition: etc.) and only report the manipulation of the DOM given by bind:this.

However, I still don't know if it should be reported if the user is using typescript and the element is specified as an argument to the function.

function fn(element: HTMLElement) {
  element.remove(); // We don't know if the `element` passed through `bind:this`.
}

[ota-meshi]

I started implementing this rule. #302
Please comment if you have any.

[baseballyama]

Thank you @ota-meshi!
Now I'm implementing svelte/Infinite-reactive-loop. (Next few days I can't take time, so I hope I will create PR in the next 2 weeks)
So please continue to implement😀