Skip to content

fix: treat new expression references in template as possibly dynamic #10636

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

Closed
wants to merge 1 commit into from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Feb 26, 2024

Fixes #10629.

This is an interesting one, but one I think worth considering. When a binding is passed into a template such as

<div>{foo}</div>

When we check if foo is dynamic, unless it comes from state or a prop, it will generally be considered as not dynamic as its just an identifier (member expressions and call expressions are always treated as dynamic). Maybe we can tweak this so that if we see it comes from a new expression that is gets treated as being possibly dynamic. That's because classes are more likely to have a custom toString that might be reactive, thus the identifier itself might be dynamic.

Copy link

changeset-bot bot commented Feb 26, 2024

🦋 Changeset detected

Latest commit: d2859a6

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

@dummdidumm
Copy link
Member

See #10629 (comment) and #10629 (comment) - we can't possibly statically analyze all cases, so there will still be false positives. I'm not sure yet if that puts me into the camp of "rather be consistent and make the gotcha obvious earler" or "try to make the gotcha appear less often" as this PR would.

@Evertt
Copy link

Evertt commented Apr 1, 2024

@trueadm @dummdidumm why not do the opposite behavior?

Why not assume that anything could be dynamic and should therefor better be rendered in the template using render_effect()?

As far as I can see, in the worst case scenario you put something in render_effect() that isn't dynamic and you lose a negligible amount of performance. Only the very first render will have a tiny bit of overhead, but after that no re-renders will happen anyway.

But on the other hand, currently there are many scenarios where a variable that was meant to be dynamic is not recognized as such, and so is rendered outside of a render_effect() and therefor is not rendered reactively. This would solve that.

@trueadm trueadm deleted the new-expression-tpl branch November 6, 2024 15:02
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

Successfully merging this pull request may close these issues.

Svelte 5: reactive date may be not reactive in the markup
3 participants