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

Implement spread binding #6162

Closed
wants to merge 2 commits into from
Closed

Implement spread binding #6162

wants to merge 2 commits into from

Conversation

sxxov
Copy link

@sxxov sxxov commented Apr 3, 2021

This implements #5137, and makes it one step closer to solve #2226

I'm pretty new to the svelte codebase, so it would be nice if there were pointers to if I did something tragically wrong. I added docs & test cases as well.

Here's a small test repo too, which code I adapted into the docs: https://github.com/Sxxov/svelte-spread-bind-test-case

edit: forgot to note the syntax I landed with — {...bind:variable}

@pushkine
Copy link
Contributor

pushkine commented Apr 4, 2021

...bind:variable is not valid javascript

@antony
Copy link
Member

antony commented Apr 4, 2021

As @pushkine states, one of the mainstays of Svelte is that it remains valid javascript, parseable with standard tooling.

For this reason, thank you for this PR but it cannot be added to Svelte in its current incarnation.

The best way to add a language feature is to first create an RFC where this sort of issue can be weeded out.

If you need spread binding for props it may be worth creating this RFC to discuss the proposal (however, needing to bind loads of things at once like this is indicative that it would be better to use a store, and the context API).

Please don't be discouraged from contributing however, for what it is worth, your PR looks great, has tests and docs, and is otherwise great :)

@antony antony closed this Apr 4, 2021
@sxxov
Copy link
Author

sxxov commented Apr 4, 2021

@antony

ahh, I wasn't aware of the valid js requirement. thanks for the RFC pointer! I wasn't sure if this needed it since the issues i snooped around didn't seem to have much discussion.

I could think of a few more scenarios other than bind forwarding (namespaced bound variables in the parent, the ability to iterate through the namespace, etc), just overall from an angle of "spread on steroids". (and wouldn't stores/context API not solve this, since binds have the ability to travel upstream, thus requiring a parent scope to house all childrens' stores and defeat the point of the scoped API in the first place?)

@Florian-Schoenherr
Copy link

@antony this needs an RFC? Seems to me like an obvious omission in the language...
but I guess due to a slight break in either js, svelte or reservation of props you're right.

@sxxov would you like to connect on discord for some back and forth about writing an RFC?

@sxxov
Copy link
Author

sxxov commented Jul 30, 2021

@Florian-Schoenherr

Here's what I've got so far sveltejs/rfcs#57, do put any comments you might have on it, or find me on twitter if ya'd just like to chat (:

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

Successfully merging this pull request may close these issues.

4 participants