Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New rule: no undefined variable in the DOM #747

Open
Ennoriel opened this issue May 2, 2024 · 7 comments · May be fixed by #807
Open

New rule: no undefined variable in the DOM #747

Ennoriel opened this issue May 2, 2024 · 7 comments · May be fixed by #807
Labels
enhancement New feature or request help wanted Extra attention is needed new rule

Comments

@Ennoriel
Copy link

Ennoriel commented May 2, 2024

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

@Ennoriel Ennoriel added enhancement New feature or request new rule labels May 2, 2024
@ota-meshi
Copy link
Member

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/

@ota-meshi ota-meshi added the help wanted Extra attention is needed label May 3, 2024
@baseballyama
Copy link
Member

baseballyama commented May 4, 2024

@baseballyama
Copy link
Member

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

@ota-meshi
Copy link
Member

ota-meshi commented May 4, 2024

I didn't know that 😅 I can’t wait for the GA of Svelte v5!!!

@Ennoriel
Copy link
Author

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
Copy link
Member

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

@AlbertMarashi
Copy link

image
This should never be allowed. The Svelte 5 solution in my opinion is actually a footgun. We need a way very much like @typescript-eslint/restrict-template-expressions to disallow non-string or non-primitive values from being serialised into [object Object] as this is a major potential bug surface for apps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed new rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants