Don't create class update functions when dependencies aren't reactive #5926
Conversation
| // to add an updater for this. | ||
| const any_dynamic_dependencies = all_dependencies.some((dep) => { | ||
| const v = this.renderer.component.var_lookup.get(dep); | ||
| return !v || is_dynamic(v); |
There was a problem hiding this comment.
Rename variable v to something meaningful, such as variable.
Beyond that, this PR is great 👍
There was a problem hiding this comment.
Oops, completely missed that! Will push a commit to fix that
| </script> | ||
|
|
||
| <!--These should all get updaters because they have at least one reactive dependency--> | ||
| <div class:update1={reactiveModuleVar}></div> |
There was a problem hiding this comment.
actually moduleVar will never be reactive as it does not belong to any component instance
There was a problem hiding this comment.
Interesting! If it's okay with you, I would like to file an issue for that and solve it in a different PR. It's being indicated as being reactive by is_dynamic and I worry that changing that function to fix this will have other effects and would prefer to isolate that (and the testing for it) to a new PR.
My PR won't cause a regression even though it treats the module var as reactive. https://svelte.dev/repl/3a6d024461d74e43b5fdf7ea144a9a80?version=3.32.0 shows that Svelte already creates a toggle_class for a module var in the update function so my PR won't change that behavior at all.
There was a problem hiding this comment.
Filed an issue at #5943 and I'll hopefully have a fix out within a week
* 'master' of https://github.com/sveltejs/svelte: (129 commits) -> v3.32.3 fix remove of lone :host selectors (sveltejs#5984) -> v3.32.2 update changelog fix extra invalidation with component prop binding to object property (sveltejs#5890) update css-tree@^1.1.2 (sveltejs#5958) fix :host and :global css scoping (sveltejs#5957) Tutorial : a better explanation of component events (sveltejs#4639) "What's new in Svelte" February newsletter (sveltejs#5925) Change color on 404 page (sveltejs#5932) -> v3.32.1 warn module variables are nonreactive and make them truly nonreactive (sveltejs#5847) update changelog fix: "foreign" namespace elements should still allow binding 'this' (sveltejs#5942) inline `prop_values` in `init` helper (sveltejs#5909) update changelog don't create class update functions when dependencies aren't reactive (sveltejs#5926) fix extraneous store subscription in SSR (sveltejs#5929) make `SvelteComponentDev` typings more forgiving (sveltejs#5937) make animation/transition params optional (sveltejs#5936) ...
Summary
Don't generate a
toggle_classupdater for a component when the dependencies for the class assignment (e.g.class:foo={dep1 && dep2}) are non-reactive.Motivation
Svelte is a compiler and a great thing about compilers is the ability to perform code analysis and optimize the code! This will reduce the bundle size of Svelte-generated code and improve it's speed by having it check less when an update occurs.
Closes #5919
Test Plan
I added a code test that makes sure updaters are still added for reactive dependencies, but that they're no longer added for non-reactive dependencies. As well as that, the following tests already exist that check related behavior: