sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.55k stars 4.12k forks source link

Svelte 5 - Relax Variable Naming Restrictions on $-Prefixed Variables #12218

Open roonie007 opened 3 months ago

roonie007 commented 3 months ago

Describe the problem

I have encountered a significant issue with Svelte 5 where all variables starting with the $ sign are reserved. This creates conflicts with common practices in various projects and organizations where $-prefixed variables are used for internal functions (e.g., $db, $session, $redis).

Here are some arguments to support the request for not reserving all $-prefixed variables in Svelte:

Describe the proposed solution

Consider reserving only specific, essential variables for Svelte's use, rather than all $-prefixed variables. This would minimize the impact on existing projects and allow developers to continue using their preferred variable naming conventions without conflict.

If the concern is about introducing breaking changes with new Svelte features, it is easier to rename a few specific variables than to block all variables with a $ prefix.

Importance

i cannot use svelte without it

trueadm commented 3 months ago

You can always import the package imports as a namespace. This is something we've been advocating for folks who have this exact problem, for example with Lexical:

import * as Lexical from 'lexical';

// ..

lexical.$insertNodes(…)
roonie007 commented 3 months ago

@trueadm I don't think you understood my point, we have internal conventions and rules and all of our projects use $ and in case we WANT to use Svelte 5, we are FORCED to break those rules.

In my opinion, a framework should absolutely not break a company or even a developer code writing rules.

I saw the other issues related to the $ sign, they suggest the removal of the $ sign from the runes, but here I am asking to relax the restriction.

trueadm commented 3 months ago

@roonie007 I know, I was the author of Lexical that also had the convention of using $ prefixes on the APIs. However, like I said above, you're not removing the convention, you're just working around it. In the case of .svelte modules, we enforce that $ prefix isn't allowed as it enables us to use those for runes in the future. It would suck if we couldn't add a $db() or a $session() rune in the future to SvelteKit – for example.

roonie007 commented 3 months ago

@trueadm That’s precisely my point. While svelte could introduce $db or $session in the future, in our codebase, we have the flexibility to rename variables as needed (e.g., $database, $mongo, $prisma, $userSession, etc...).

Having the choice is always preferable to being restricted. Allowing us to continue using $-prefixes for our internal functions without enforcing a blanket restriction would provide more flexibility and avoid unnecessary workarounds.

paoloricciuti commented 3 months ago

@trueadm That’s precisely my point. While svelte could introduce $db or $session in the future, in our codebase, we have the flexibility to rename variables as needed (e.g., $database, $mongo, $prisma, $userSession, etc...).

Having the choice is always preferable to being restricted. Allowing us to continue using $-prefixes for our internal functions without enforcing a blanket restriction would provide more flexibility and avoid unnecessary workarounds.

The problem is semver: if you don't restrict $ variables whenever you want to add a new rune that's a breaking change. So while you absolutely can rename your variables svelte can't publish a new rune without bumping to svelte 6 (which would be kinda restrictive)

rChaoz commented 3 months ago

Here's a suggestion: warn (not error) on variable/import with name of rune, and disable the rune for the entire file. So you can do import $state from "somewhere" and it gives you a warning:

The import/variable name clashes with a Svelte rune. The $state rune cannot be used in this file unless the import/variable is renamed.

The warning should maybe be able to be suppressed.

This way new runes are not breaking changes. And the existing error for all $-prefixed variables should be removed, or at least changed to warnings (which should definitely be able to be suppressed).

roonie007 commented 3 months ago

@rChaoz suggestion is also an acceptable solution to relax the restriction.

rChaoz commented 1 month ago

Could this issue be considered for the 5.0 milestone, at least to be discussed? There's a chance that fixing this might be breaking, depending on the preferred solution