Skip to content

[fix] don't run binding init unnecessarily #7981

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 5, 2022
Merged

[fix] don't run binding init unnecessarily #7981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/compile/render_dom/wrappers/InlineComponent/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export default class InlineComponentWrapper extends Wrapper {

component.partly_hoisted.push(body);

return b`@binding_callbacks.push(() => @bind(${this.var}, '${binding.name}', ${id}));`;
return b`@binding_callbacks.push(() => @bind(${this.var}, '${binding.name}', ${id}, ${snippet}));`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what sorts of situations will ${snippet} be an expression that is undefined vs non-undefined? Is this just an approximation of whether it's the first call or not, and we're accepting that ${snippet| might be undefined some other times, at which point we'll call callback when ideally we wouldn't have to? Or is something else going on?

Copy link
Member Author

@dummdidumm dummdidumm Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In

<script>
  let foo = "bar";
</script>

<SomeConponent bind:foo />

snippet is the value of foo, so it's defined in this case as bar, and would be undefined if foo wasn't initialized, at which point we want to assign foo to the value that foo has inside SomeComponent.

I don't know why it worked before having the correct value for foo (being bar, not the value of foo inside SomeComponent), but having this check breaks the infinite loop in the related issues.

});

const munged_handlers = this.node.handlers.map(handler => {
Expand Down
6 changes: 4 additions & 2 deletions src/runtime/internal/Component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import { children, detach, start_hydrating, end_hydrating } from './dom';
import { transition_in } from './transitions';
import { T$$ } from './types';

export function bind(component, name, callback) {
export function bind(component, name, callback, value) {
const index = component.$$.props[name];
if (index !== undefined) {
component.$$.bound[index] = callback;
callback(component.$$.ctx[index]);
if (value === undefined) {
callback(component.$$.ctx[index]);
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions test/runtime/samples/binding-no-unnecessary-invalidation/Tab.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script>
export let tab;
</script>
7 changes: 7 additions & 0 deletions test/runtime/samples/binding-no-unnecessary-invalidation/_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default {
async test({ assert, target }) {
assert.htmlEqual(target.innerHTML, `
<p>0</p>
`);
}
};
17 changes: 17 additions & 0 deletions test/runtime/samples/binding-no-unnecessary-invalidation/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<script>
import { writable } from "svelte/store";
import Tab from "./Tab.svelte";

let i = 0;
const { set, subscribe } = writable({ id: 1, name: "tab1" });
const tab = {
set(value) {
i++;
set(value);
},
subscribe,
};
</script>

<Tab bind:tab={$tab} />
<p>{i}</p>