-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
"current data object" in templates (re: spread) #1303
Comments
One thing about template tags — You can't, however, use it in expressions (even So I wonder if the right move here is to make <h2>{{this.thing}}</h2>
{{#each things as thing}}
<p>What is {{this.thing}}?</p>
{{/each}} It sort of looks like the second instance would resolve to |
👍 I think this would be the least surprising behavior. I can't think of much reason to use it outside of spread, but that would feel consistent to me.
One of the things that I really really like about Svelte's mustache implementation compared to Ractive is that I never have to think about scopes created by different mustache features. The only option is to I feel pretty strongly that I want Svelte to stay this way, and never create new scopes - devs should be forced to reference
For one thing, unless you made |
Question unrelated to the previous posts: are we cool with the idea that |
(I think having |
💯 yes, it would be deeply confusing if it had a dual meaning. My earlier point about |
oh, I follow you now. Yeah, I would expect it/want it to reference the top-level/component state :-x Otherwise, I can't use spread in loops, and we're in the I-have-to-think-about-what-scope-I'm-in territory where I don't want to be. How unfortunate are the hoops? >_< |
So would using <h2>Calendar Events</h2>
{{#each events as event}}
<div>
<h4>{{ event.name }}</h4>
<button on:click="cancelEvent(this.event)">Cancel</button>
</div>
{{/each}} |
Well, #1173 changed the behaviour so that instead of passing down a
well... no. <input on:focus='this.select()' on:input='updateValue(this.value)' > |
Maybe we can unite this feature with one more idea - creating computed properties which depend on all data. We can use computed: {
props(...data) => data
} And after that <Child {{...props}} /> It'll also allow to pass not all data and filter it somehow: computed: {
props(...data) => Object.keys(data).reduce(() => {}, {})
} Just an idea. |
I do think that the 'computed properties with a rest param' thing is an interesting idea, but there are some things we need to be careful of. It seems like there are two main ways it could work:
The second option is what I was focusing on in a brief Gitter conversation, but I'm now thinking that the first one is better. It's less confusing to understand (try writing docs that don't suck for that second one!), and also it's not any less powerful. If someone wants a rest-param-using-computed-property to depend on the computed properties, they can just list it as an explicit dependency with its own argument. The list of computed properties is known at compile time to Svelte, and will also be known at coding time to the developer, so there doesn't really seem to be any reason to include computed properties in the rest param. |
Completely agree. Perhaps, Store data also should be explicit: computed: {
foo(bar, $baz, ...data) {
},
bar(qux) {
}
} |
What I understand is that anything we (as a community) define here will probably have two implications:
So I will suggest something that may happen to be more "friendly" and customizable to everyone, but it is just an idea, so maybe it is just bad (but to me it makes sense). Why just do not add a new option, on the object exported by
As an example, I see something like that happening:
I do not have an idea actually about the name used to define the option, but I do not see a problem on the approach actually. Of course, let configuration happen that way depends totally on the architecture of the compiled (which I do not know anything about). Another option would be to define that option as a option to What do you think? Is that a viable option? (Thanks for creating Svelte, by the way, I absolutely love this project!) |
I made a note in #1338, but it occurs to me that this problem is solved by #1069: <Widget {...props}/>
<script>
import Widget from './Widget.html';
export default {
components: { Widget },
computed: {
props: state => state
}
};
</script> This gives you flexibility over the naming, adds no new syntax, and allows you to exclude props you don't want: props: state => {
const { unwanted, alsoUnwanted, ...props } = state;
return props;
} |
How would this handle regular vs computed properties? Computed properties can depend on other computed properties, but things seem they would get hairy if the |
I assume that the concept of "what is available in the state for computed properties" would stay the same as it is now - containing both raw values, and computed properties. I write lots of computed properties that depend on other computed properties. |
At the moment, the compiler uses its knowledge of each computed property's dependencies to sort them topologically: // `b` will be computed before `c`
computed: {
c: ({ b }) => b * 2
b: ({ a }) => a * 2
} If a computed property accepts the whole state object, it just goes at the end. If you have two or more such properties, they could get out of order if one depends on the output of another, but I think the solution to that is a) document it, and b) preserve the order in which those properties were specified, so it's still deterministic. |
well, I like the proposed idea. With the release of the version 2, what blocks this issue from being implemented with the proposed idea? just to know |
nothing! someone just needs to make it happen. I'd probably get round to it eventually, but if anyone wants to make a PR... 😀 |
I had always assumed they were processed in the order they appeared, so I
think that makes sense.
|
So I'm guessing then that we would not allow a computed property that explicitly depends on a computed property that depends on the whole state object? (That would prevent computed properties that accept the whole state object from all going at the end.) But I'm still not sure what's a good way to handle passing other (normal or whole-state) computed properties to a whole-state computed property. Would each one simply get the whole state object as it currently exists, whether its current mix of updatedness and nonupdatedness? I'm imagining that would be simplest to implement, but may be confusing to use. |
If whole-state computed properties get called after everything else, then the only issue would be other whole-state computeds. There's probably no way to magically resolve how whole-object computed properties see each other - I think as long as they were resolved deterministically in order of declaration it would be fine. Having multiples of this special case computed property would be pretty uncommon I imagine. |
What if we don't do any ordering based on dependencies? Just process them in the order they are defined. This puts all the power into the hands of the developer, as well as make it explicit and clear how it works. Otherwise we are left guessing what might happen in this situation or that situation and may lead to difficult bugs to resolve. {
computed: {
props: state => ({ ...state }), // `props.name` may be stale
name: ({ firstName, lastName }) => firstName + ' ' + lastName,
props2: state => ({ ...state }), // `props2.name` will not be stale
}
} It is easy to look at the computed property and follow through what happens from top to bottom. Just like event listeners, the first ones are called first, then the second, etc. We can try to "fix" things to help you avoid bugs, but then it makes it more complex and less understandable how it works, possibly causing bugs that you can't understand or easily fix. Though, this is theoretical, I have no practical examples lying around. I do like the simplicity of top to bottom however. |
There is no way I wouldn't shoot myself in the foot with that behavior :-x I don't think there's any benefit to ever passing stale values to a single whole-state-computed function. |
Well, there isn't value in stale values, just in a consistent model. I'll backtrack here. I agree with your (@TehShrike) earlier comment about putting those properties last and going in order of declaration. |
implement full-state computed properties
This is available as of 2.3.0: https://svelte.technology/repl?version=2.3.0&gist=f086da6a9c502c6f86abfa8b70eb88ca (note that Acorn doesn't like object rest syntax, so you can't currently use that in your computed properties) |
Really? What exactly doesn't work? I'm using object spread/rest with (non-Svelte) things that are being bundled by Rollup. Once object spread/rest reached stage 4 of officially being in ES2018, they were implemented in Acorn I believe. |
Does this work with |
No. Perhaps we could do that by omitting the second argument: store.compute('y', state => state.x * 2); |
Follow-on to #195 -
In order to use the spread functionality to pass arbitrary properties that were applied to the current component on to child components, we need to be able to reference the current data/state object in the template.
I think
{{...state}}
and{{...data}}
and{{...@}}
were tossed around, but my vague impression is that{{...this}}
is the current top pick.The text was updated successfully, but these errors were encountered: