-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore: delete weird code #9571
chore: delete weird code #9571
Conversation
🦋 Changeset detectedLatest commit: 01428b3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I don't think we can remove this code. It guards against static things becoming accidentally reactive because they happen to be on the same text node as an actually reactive variable. We should add a test for this. |
Huh. I didn't realise we were doing that. It seems pretty weird to be honest. Why would we use the initial value in render effects (assuming we were in fact doing that, and not the current thing where we only do that if it's a standalone This is the current behaviour on In my view it would be much better to print a 'hey you probably didn't mean this' warning if
but otherwise just let it update at random times. I don't think the current behaviour matches any plausible user intent. |
I vaguely remember that this was built in when we were more broad with how changes result in view changes, i.e. one render effect for everything. That made people raise pitchforks, questioning how this is fine-grained reactivity when it now updates more broadly in certain situations than Svelte 4. |
no idea what this highly suspicious code is supposed to be doing, but no tests fail if we delete it. closes #9556
Svelte 5 rewrite
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 notmain
.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint