Skip to content
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

[fix] propagate bindings correctly #8114

Merged
merged 1 commit into from
Jan 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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}, ${snippet}));`;
return b`@binding_callbacks.push(() => @bind(${this.var}, '${binding.name}', ${id}));`;
});

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

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

Expand Down
25 changes: 20 additions & 5 deletions src/runtime/internal/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,32 @@ export function add_flush_callback(fn) {
const seen_callbacks = new Set();
let flushidx = 0; // Do *not* move this inside the flush() function
export function flush() {
// Do not reenter flush while dirty components are updated, as this can
// result in an infinite loop. Instead, let the inner flush handle it.
// Reentrancy is ok afterwards for bindings etc.
if (flushidx !== 0) {
return;
}

const saved_component = current_component;

do {
// first, call beforeUpdate functions
// and update components
while (flushidx < dirty_components.length) {
const component = dirty_components[flushidx];
flushidx++;
set_current_component(component);
update(component.$$);
try {
while (flushidx < dirty_components.length) {
const component = dirty_components[flushidx];
flushidx++;
set_current_component(component);
update(component.$$);
}
} catch (e) {
// reset dirty state to not end up in a deadlocked state and then rethrow
dirty_components.length = 0;
flushidx = 0;
throw e;
Comment on lines +75 to +78
Copy link
Member

Choose a reason for hiding this comment

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

Can you tell an example that comes to the catch statement?
Is it intended to handle Maximum call stack size exceeded?

Is there any possibility that this may cause the unintended DOM structure and the runtime to go wrong behavior?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's on test that throws a runtime error (I believe some dev time check for svelte:element) and then all other tests afterwards fail because flush is in a broken state then. Yes, we could end up in a broken Dom state but at least the app might continue to work. The alternative would be to not do this but only do console.error in that check.

}

set_current_component(null);

dirty_components.length = 0;
Expand Down
6 changes: 6 additions & 0 deletions test/runtime/samples/binding-indirect-value/Component.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
export let value;
value = "bar";
</script>

Child component "{value}"<br />
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
export default {
async test({ assert, target }) {
assert.htmlEqual(target.innerHTML, `
<p>0</p>
Parent component "bar"<br />
Child component "bar"<br />
`);
}
};
8 changes: 8 additions & 0 deletions test/runtime/samples/binding-indirect-value/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Component from "./Component.svelte";

let value = "foo";
</script>

Parent component "{value}"<br />
<Component bind:value />
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// this test currently fails because the fix that made it pass broke other tests,
// see https://github.com/sveltejs/svelte/pull/8114 for more context.
export default {
skip: true,
async test({ assert, target }) {
assert.htmlEqual(target.innerHTML, `
<p>0</p>
`);
}
};