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

Loop protect guard issue in REPL #4034

Closed
bwbroersma opened this issue Dec 2, 2019 · 5 comments
Closed

Loop protect guard issue in REPL #4034

bwbroersma opened this issue Dec 2, 2019 · 5 comments
Assignees
Labels

Comments

@bwbroersma
Copy link
Contributor

Describe the bug
The bind this example doesn't work in 3.16.0 (it does in 3.15.0)

To Reproduce (minimal example)

<script>
  let node;
  (function () {
    while(false) ;
  }());
</script>
<div bind:this={node}></div>

Produces guard is not defined.
See REPL (it does work on 3.15.0).
Note that the bind:this is actually needed to trigger this. It's probably a visibility issue with the guard and an anonymous function?

@Conduitry Conduitry added the bug label Dec 2, 2019
@Conduitry
Copy link
Member

In the generated code:

    function instance($$self, $$props, $$invalidate) {
    	let node;

    	(function () {
    		const guard = loop_guard(100);

    		while (false) {
    			
    			guard();
    		}
    	})();

    	function div_binding($$value) {
    		binding_callbacks[$$value ? "unshift" : "push"](() => {
    			$$invalidate(0, node = $$value);
    		});
    	}

    	$$self.$capture_state = () => {
    		return {};
    	};

    	$$self.$inject_state = $$props => {
    		if ("node" in $$props) $$invalidate(0, node = $$props.node);
    	};

    	return [node, guard, div_binding];
    }

guard is being included in the ctx array, and it should not. It seems to happen with any kind of bind:, not just bind:this.

@bwbroersma
Copy link
Contributor Author

@Conduitry: I couldn't get a setup outside of the REPL that generated a guard, what's the trick? (I tried with svelte({dev: true}))

@Conduitry
Copy link
Member

You also need to pass a non-zero loopGuardTimeout option to the compiler.

@Conduitry
Copy link
Member

Conduitry commented Dec 2, 2019

The problem seems to be the this.add_var() call in Component#loop_protect(). We don't want to use add_var() because that adds it to the context, and the guard function should not be part of the component's context. Simply removing that, though, breaks the loop-protect tests - there's a failure in Component#rewrite_props() because we can't find the guard variable in var_lookup so something else needs to happen. Why the walker is even getting to this node, which shouldn't be in the top-level scope, I do not know.

Edit: Ohhhh. In the example in this issue, it's in a deeper scope. But in the loop-protect test, the loop is at the top level. Duh. I think we can just wrap the whole loop in a { } scope so that the guard variable doesn't leak out.

cc @tanhauhau

@Conduitry
Copy link
Member

Fixed in 3.16.1 - https://svelte.dev/repl/bind-this?version=3.16.1 - and will work again on the main examples page once unpkg's cache is refreshed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants