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

Disallow props that look like directives #6823

Closed
benmccann opened this issue Oct 8, 2021 · 11 comments
Closed

Disallow props that look like directives #6823

benmccann opened this issue Oct 8, 2021 · 11 comments

Comments

@benmccann
Copy link
Member

Describe the problem

Currently every directive we add is technically a breaking change, because someone could have already (for some reason) been using a prop/attribute called bubble:foo or whatever.

Describe the proposed solution

Pick a format for directives (probably what we already have now) and make it a compile-time error to have a prop that looks like a directive but doesn't match the names of any of them.

Alternatives considered

No

Importance

nice to have

@benmccann benmccann added this to the 4.x milestone Oct 8, 2021
@Conduitry
Copy link
Member

This should also apply to attributes that look like directives. I don't know how to make this play nicely with https://kit.svelte.dev/docs#anchor-options - I don't want SvelteKit-specific escape hatches in Svelte. We'll need to cross that bridge at some point.

@kevmodrome
Copy link
Contributor

This should also apply to attributes that look like directives. I don't know how to make this play nicely with https://kit.svelte.dev/docs#anchor-options - I don't want SvelteKit-specific escape hatches in Svelte. We'll need to cross that bridge at some point.

Maybe a pre-processor in SK to get around this?

@TheCymaera
Copy link
Contributor

How about a special syntax like this?

<div @on:click={...}></div>

This would be a big breaking change.

@rayl0
Copy link

rayl0 commented Jan 18, 2023

How about a special syntax like this?

<div @on:click={...}></div>

This would be a big breaking change.

I think this should be done, instead of other one suggest above

@rayl0
Copy link

rayl0 commented Jan 18, 2023

I don't know if they work the way they work but I use tailwindcss for almost all the css side of things.
So, it would be impossible for me to use class directives with tailwindcss
<div class="bg-white px-20 py-20" class:dark:bg-gray-400={toggleTheme} class:hover:hidden={isHidden}></div>

@dummdidumm
Copy link
Member

I don't know if they work the way they work but I use tailwindcss for almost all the css side of things. So, it would be impossible for me to use class directives with tailwindcss <div class="bg-white px-20 py-20" class:dark:bg-gray-400={toggleTheme} class:hover:hidden={isHidden}></div>

IMO this should be ok, since the attribute name as seen by Svelte is class:.. and class: is a reserved keyword for Svelte, so it knows to interpret things afterwards as the class name.

This should also apply to attributes that look like directives. I don't know how to make this play nicely with https://kit.svelte.dev/docs#anchor-options - I don't want SvelteKit-specific escape hatches in Svelte. We'll need to cross that bridge at some point.

Thankfully we made the decision to have data-sveltekit-X for this, so we should be fine. There's the deprecated Sapper, but it likely only works with Svelte 3 anyway, so we shouldn't worry about that.

I propose to make this a warning in Svelte 4 and an error in Svelte 5.

@Conduitry
Copy link
Member

I propose to make this a warning in Svelte 4 and an error in Svelte 5.

Would it being a warning be enough for us to feel safe adding new directives in some 4.x after 4.0? Or do you think we shouldn't worry about strict semver compliance here, because we're already non-compliant in 3.x and emitting warnings is a step in the right direction?

@dummdidumm
Copy link
Member

I'd say so, yes. Also I think it's unlikely we add new directives in 4.x, it's more likely in 5.x

dummdidumm added a commit that referenced this issue May 25, 2023
...to prevent ambiguity with Svelte directives

closes #6823
dummdidumm added a commit that referenced this issue May 26, 2023
closes #6823

---------

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <git@rich-harris.dev>
@benmccann
Copy link
Member Author

There's a warning for these in v4

@farfromrefug
Copy link

@benmccann is there a way to disable that specific warning? In svelte-native we use prop: and ios: and android:. Maybe there is a way to say those are ok?

@dummdidumm
Copy link
Member

Could you open a new issue so we can track this more visibly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants