sveltejs / eslint-plugin-svelte

ESLint plugin for Svelte using AST
https://sveltejs.github.io/eslint-plugin-svelte/
MIT License
276 stars 30 forks source link

New rule: `no undefined variable in the DOM` #747

Open Ennoriel opened 2 months ago

Ennoriel commented 2 months ago

Motivation

When using a variable (say a props) that can be null or undefined directly in the DOM, it will be rendered as a string "null" or "undefined". Hence, it is probablly never going to be a good thing to use a possibly null or undefined variable directly in the DOM.

Description

The rule would highlight any variable or expression used in the DOM that resolves to a type involving a union type to null or undefined.

Examples

<script lang="ts">
  export let prop: string | undefined = undefined;

  const frameworks = ["svelte", "react"]
</script>

<!-- βœ“ GOOD -->
{#if prop}
  {prop}
{/if}

<!-- βœ— BAD -->
{prop}
{frameworks.find(framework => framework === prop)}

Additional comments

No response

ota-meshi commented 2 months ago

Thank you for posting the rule suggestion. That rule is sounds good to me! However, it may be useful for more users to extend the rule further and make it a check rule similar to @typescript-eslint/restrict-template-expressions. What do you think? https://typescript-eslint.io/rules/restrict-template-expressions/

baseballyama commented 2 months ago

Svelte 5 solved this issue. https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE0WKzQoCIRDHX0X-Z6G7bUHPkR1CRxjYnRHHDUJ89_DU8fcxUHgnQ3gOyPsgBDxqhUf_1gX2ob0TPEzPlpbZLDWu_R4l9qRi3RVVd3OnZCoslK9Rtst_kpXZXMQoqjMCHodmLkwZobeT5mv-AJMTcguHAAAA

So should we implement rules for Svelte 4 now that Svelte 5 is RC?

baseballyama commented 2 months ago

I think what users need to do is migrating the app to Svelte 5 instead of adding the rule.

ota-meshi commented 1 month ago

I didn't know that πŸ˜… I can’t wait for the GA of Svelte v5!!!

Ennoriel commented 1 month ago

Thanks for pointing out this evolution is Svelte 5.

I am concerned whether this is a better solution. Usually, when I had undefined or null rendered, I had to add a #if block not only around the variable displayed but around some dom elements (<p>name: {name}</p>).

Maybe it's still necessary to have this rule but as a warning and not an error?

ota-meshi commented 1 month ago

Yeah, I think the priority has been lowered, but someone might want to make sure it's explicitly a string.