sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.47k stars 4.19k forks source link

Disallow prop interpolation + other stuff without quotes? #12910

Closed brunnerh closed 2 months ago

brunnerh commented 2 months ago

Describe the bug

You can do stuff like this:

<Component
  value={i}
  valueWithSuffix={i},
  values={i}{i2}
/>

It leads to less than optimal results. Things like the added , are likely just a typo which is currently not flagged but just coerces concatenation between value and string ",".

Reproduction

<script>
    import Component from './Component.svelte';
    const i = 1;
    const i2 = 1;
</script>

<Component
    value={i}
    valueWithSuffix={i},
    values={i}{i2}
/>
<!-- Component.svelte -->
<script>
    const props = $props();
</script>
<pre>{JSON.stringify(props, null, 2)}</pre>

Output in Svelte 5:

{
  "value": 1,
  "valueWithSuffix": "1,",
  "values": "11"
}

Output in Svelte 4 (different!):

{
  "value": 1,
  "valueWithSuffix": "1,",
  "values": "2"
}

Logs

No response

System Info

REPL

Severity

annoyance

Conduitry commented 2 months ago

I'm undecided on how I think we should handle this discrepancy. Forbidding code like this in 5 is probably the simplest to document, yeah.

{foo}{bar} resulting (in 4) as foo + bar and being a sum rather than a string concatenation if they're both numbers is the only behavior here that I think really irks me, though, and that's been resolved in 5. {foo}x being the same as "{foo}x" doesn't bother me as much, and doesn't feel to me like something we definitely want to prevent.

david-plugge commented 2 months ago

As components are indeed not valid HTML elements and we can pass values other than strings i would actually prefer to always require a single wrapping set of curly brackets. This makes it more clear that any JS value can be passed. Shorthands like passing in a string by using quotes and omitting the value to set a boolean are very nice, but other than that it feels very confusing. What would happen Here: onsomething={fn1}{fn2}? The result would probably be the string representation of 2 functions. Pretty sure it would show an error but this still feels odd. Either use {'int${'erpo'}lation'} or "the {'svelte way'}".

Whatever will be done, beeing able to require this more strict way by defining an eslint rule is fine too.

Rich-Harris commented 2 months ago

Wow I had no idea about the {i}{i2} thing in Svelte 4. 'Resolved' or not, I'm strongly in favour of disallowing concatenation without quotes.

dummdidumm commented 2 months ago

Prettier auto-formats this code to use quotes, so I'm not sure how much of a problem this is in practise

Rich-Harris commented 2 months ago

According to the National Geographic, some tribes in the Amazon basin still don't use Prettier. But closer to home, even we don't use it in the REPL (though it would be nice if we did, somehow). I think it's still worth disallowing - it aligns with our plans regarding quoted props more broadly, and it feels like most of the time it's probably a typo

Rich-Harris commented 2 months ago

Hang on a minute. We already error on this — #11736 — we just only do it in runes mode, so that it's not an immediate breaking change when you upgrade.

I almost wonder if we should reinstate the {1}{1} === 2 behaviour, so that we're bug-for-bug compatible... probably not worth it though.

For now I've just opened #12988 to add a note in the docs, I don't think there's anything else we need to do here.

dummdidumm commented 2 months ago

As in ... close this issue?

Rich-Harris commented 2 months ago

yes — will be auto-closed by #12988