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

$props is not defined thrown with dynamic type-attribute "workaround" #4368

Closed
arggh opened this issue Feb 4, 2020 · 4 comments · Fixed by #4453
Closed

$props is not defined thrown with dynamic type-attribute "workaround" #4368

arggh opened this issue Feb 4, 2020 · 4 comments · Fixed by #4453
Labels
awaiting submitter needs a reproduction, or clarification bug

Comments

@arggh
Copy link
Contributor

arggh commented Feb 4, 2020

Prologue

With Svelte, it's somewhat cumbersome to create a typical <TextField> or <Input> -component with a dynamic attribute for type.

A workaround is required to get two-way bindings working, and the first one I found was the one being used at c0bra/svelma:

<script>
export let value;

const onInput = e => {
   value = e.target.value
   $$props.value = value
}
</script>

<input on:input={onInput}/>

Describe the bug
The pattern described above worked fine in Svelte 3.10.0 and earlier, but with 3.10.1 and later, an error is thrown on every input-event:

Uncaught ReferenceError: $props is not defined at HTMLInputElement.onInput

The value binding seems to work regardless, though.

To Reproduce

I took the <Input>-component from Svelma and tweaked it a tiny bit to get it running in the REPL:

(Broken with 3.18.1)
https://svelte.dev/repl/e84cf43befae4309927fad538078fd30?version=3.18.1

(Broken with 3.10.1)
https://svelte.dev/repl/e84cf43befae4309927fad538078fd30?version=3.10.1

(Working with 3.10.0)
https://svelte.dev/repl/e84cf43befae4309927fad538078fd30?version=3.10.0

Expected behavior
I'd expect not to get any errors, or a nicer, Sveltier way to achieve dynamic type-attributes.

Severity
Given how annoying it is to work around the dynamic type-attribute limitation, and then the workaround doesn't work, it definitely doubles the annoyance, if not triples?

@PatrickG
Copy link
Member

PatrickG commented Feb 4, 2020

Why do you even update $$props.value?

const onInput = e => {
   value = e.target.value;
};

Should be enough.

But I have to admit that it is a bit strange that its "allowed" to fetch properties from $$props, while the compiler thinks $props is a store when setting a property on $$props.

@arggh
Copy link
Contributor Author

arggh commented Feb 5, 2020

I honestly have no idea. I just looked up a solution to a problem in a somewhat popular Svelte UI library and found that it throws errors on each keystroke.

But you seem to be correct, it works without the $$props assignment.

The fact that this code used to work in 3.10.0 and now it throws, still stands.

@Conduitry
Copy link
Member

It appears that as of 3.10.1, assignments to $$props are treated as though you are attempting to call .set() on a store called $props.

I'm undecided on whether we should allow changing $$props. It does seem a bit dangerous, and it doesn't look like doing so invalidated things the way one might have wanted, even in 3.10.0. But if we do allow it, it shouldn't be trying to update a store called $props.

@Conduitry
Copy link
Member

I'm putting off deciding what should happen when assigning to $$props, but for now at least the exception here is no longer happening in 3.19.1 - https://svelte.dev/repl/e84cf43befae4309927fad538078fd30?version=3.19.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants