feat: error on invalid component name

[paoloricciuti]

Svelte 5 rewrite

Cheap follow up to #12798 that i've quickly implemented after a brief discussion on sveltejs/kit#12573 (comment)

Given it's already allowed for uppercase components i think it makes sense to also allow tags like arr[0]. If we actually don't want this feel free to close it didn't take me much.

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 5b105ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

Per comments elsewhere, I think we need to go in the opposite direction and only allow dot notation. This works unintentionally and is buggy

[paoloricciuti]

Per comments elsewhere, I think we need to go in the opposite direction and only allow dot notation. This works unintentionally and is buggy

Ok so by fixing you mean disallow array access even on uppercase components? Because i think we could make it work properly in the uppercase case too.

[Rich-Harris]

We definitely could make it work, but where do you draw the line?

<Things[currentThing??'default']??Fallback {...props} />

Rather than creating the illusion that any JS expression is valid, we should just have clear rules — a component name can be an identifier, or a member expression with dot notation

[paoloricciuti]

We definitely could make it work, but where do you draw the line?

<Things[currentThing??'default']??Fallback {...props} />

Rather than creating the illusion that any JS expression is valid, we should just have clear rules — a component name can be an identifier, or a member expression with dot notation

Fair enough...should i switch this PR to throwing an error in case the component is not in the right format?

[FoHoOV]

Just my two cents. Something that is not component shouldn't be allowed to be called like <X ... />.
see this for instance.

<script>
	let Component = ()=>console.log("hi");
</script>

<Component />

[7nik]

Just my two cents. Something that is not component shouldn't be allowed to be called like <X ... />. see this for instance.

<script>
	let Component = ()=>console.log("hi");
</script>

<Component />

Components aren't signed like snippets and don't really differ from any other function, so the runtime distinction is tricky and fragile. And it looks more like aiming their own leg, so...

Also, TS completely prevents this, unless Component is any or something.

[Rich-Harris]

Fair enough...should i switch this PR to throwing an error in case the component is not in the right format?

yes please

Also, TS completely prevents this

Exactly — this is what TypeScript is for. We don't need to do anything beyond that

[paoloricciuti]

yes please

@Rich-Harris did it...not entirely convinced on the error message. Also i've thrown directly in the parsing phase because if we know that it's already wrong why even continue parsing and i did consider it a parsing error rather than a validation one but don't know if we want to move it to the validation phase to keep the validation all in the validation phase.

[Rich-Harris]

tweaked the error message:

• we don't need to include the name in the message, it's already apparent from the location of the error
• 'identifier' and 'member expression' are AST jargon that not everyone understands. 'valid variable name' is something that more people will be familiar with
• messages shouldn't end with a .

[Rich-Harris]

One thing we're still missing — validation for things like <foo[1]>. This is an invalid tag, but because it isn't identified as a component name, it's not identified as an invalid component name. Not 100% sure how that should be handled, so won't attempt to fix it in this PR, but will open a new issue

[paoloricciuti]

One thing we're still missing — validation for things like <foo[1]>. This is an invalid tag, but because it isn't identified as a component name, it's not identified as an invalid component name. Not 100% sure how that should be handled, so won't attempt to fix it in this PR, but will open a new issue

I specifically avoided that validation because technically it works...the browser will create an element with foo[1] which is an inline element with no styles or semantic meaning.

I understand that this might be misleading but also since you can't have a component with array access either maybe is less of expected that this will yield a component in the first place.

[paoloricciuti]

tweaked the error message:

Yeah I wasn't really sure about the error message either for the same reasons...thanks for taking care of it.