-
Notifications
You must be signed in to change notification settings - Fork 545
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
Amendment proposal to Function-based Component API #63
Comments
I think that's a great way to mitigate the confusion around |
I was also hesitant about the use of 'value' (because it is also used as the reactive property of a now-binding) and 'state' (because it is very commonly used as a variable name), so I think this is a welcome change. Personally I'm really excited about this API. Beyond that though, I question why there are separate const state = reactive({
count: 0
}, true) // setting to true wraps each member in a binding and allows the object to be spread and retain reactivity Is there a use-case where you would expose the whole reactive object as a binding as well as its members? i.e. why might someone do this? return {
state,
...toBindings(state)
} I can't see the advantage of an extra function other than "just in case". Another drawback which I've seen raised, which is closely related to this API (i.e. exposing the reactive variables to the render context) is that this is more verbose because of the need to 1) declare the variables and then 2) expose the variables. This is a very small thing, so it's certainly no deal-breaker, but is there a way around this? I asked this in the other thread actually but it got lost (it relates directly to this API): A few more questions that aren't clear to me from the RFC:
|
@tigregalis With |
@tigregalis if you directly create an object of bindings, you'd have to access internal values as |
@Akryum I'm not sure what you mean, sorry. My comment was more around the ergonomics of spreading and exposing reactive state to the render context. |
Wouldn't Also, I personally find it very frustracting that you have to remember which one is which and always keep that in mind when working with reactive values. React has solved this very elegantly with two exports: getter and a setter. I'd much rather have this, then constantly check if I'm working with a binding or with a reactive object. const [counter, setCounter] = toBinding(0);
const increment = () => setCounter(++counter);
return { counter, increment };
|
The need for
In your example, |
@yyx990803 I haven't worked with proxies so excuse my ignorance, but is it possible to forward the |
@tigregalis spread internally uses |
Yes, I've forgotten to add that Maybe with a const counter = binding(0);
console.log(counter); // uses valueOf()
const increment = () => counter.value++; // as initially proposed
return { counter }; |
@CyberAP |
What if this return {
...toBindings(state), // retains reactivity on mutations made to `state`
double,
increment
} would automatically be done if you did this: return {
state, // retains reactivity on mutations made to `state`
double,
increment
} ie. if you directly set state in the object |
@dealloc we did discuss that option internally, but overall we want to avoid "magic keys" and be as explicit as possible. |
@yyx990803 could you please elaborate more on the non-primitive values in |
Not a very mature thought, but since import { reactive, computed, injected, merge } from 'vue'
setup() {
const state = reactive({
a: 0,
b: 1,
})
// computed accepts an object, not function,
// so that the returned computedState doesn't need to be wrapped into `computedState.value`
const computedState = computed({
c: () => state.a + 1,
d: () => computededState.c + state.b,
})
// same for injectedState
const injectedState = injected({
e: ...
})
// { state: {a, b}, computedState: {c, d}, injectedState: {e} }
return { state, computedState, injectedState }
// or
// merged into { a, b, c, d, e }, still reactive, no .value needed.
return merge(state, computedState, injectedState)
} If this is feasible, I can see two major advantages:
|
@CyberAP as mentioned, anything returned from a computed property / dependency injection has to be a binding to retain reactivity. These bindings may contain any type of values. |
@CyberAP Also there is value in using bindigs. Take those examples: https://github.com/vuejs/function-api-converter/blob/5ac41d5ad757f1fb23092e33faee12a30608a168/src/components/CodeSandbox.vue#L52-L53 It would make such usage more complicated since we would have to pass the entire object to keep reactivity. And also document each time what keys should be used or even provide accessor "callbacks"... |
A blind man asking what might be a stupid question...... If the objective is to make both object and primitive (and non-primitive) assignments reactive, couldn't it be just one method for both and have the method.... Btw, I love you are trying to make the value and state methods a bit more elegant. Thanks for that!!! Edit: Oh, and if it is possible, then the Scott |
@yyx990803 But if it were possible(?): The object would be represented by: // `state`
{
count: {
value: 0
}
} In the After spreading the So in the component template, you could still do Does any of that sound right? |
@beeplin this seems to create more API surface (another category of "options" for the return value of setup() {
const state = reactive({ ... })
const computeds = {
foo: computed(...),
bar: computed(...)
}
const injected = {
baz: inject(...)
}
const methods = {
qux() {}
}
return {
...toBindings(state),
...computeds,
...injected,
...methods
}
} A |
@tigregalis with getter forwarding, when spread into the render context, Put it another way - the render context only receives a plain number (instead of a "binding", which is trackable via the |
@yyx990803 Yes I know the 'grouping by type' thing can be done like you said. But more importantly:
If we make So, no I don't think I am an expert on proxy or JS reactivity, so I might be wrong. |
In that case I'm thinking that import { reactive } from 'vue';
export default (counter) => {
const data = reactive({ counter });
return { ...data }
} |
@beeplin that won't work when you start to extract logic into composition functions. No bindings means you will always be returning objects even in extracted functions - when you merge them you won't see the properties they exposed, and it becomes mixins all over again. |
@yyx990803 would it be technically possible (keeping all the reactivity, TS support etc) to create a reactive data sets that contain computeds and methods as well? const state = reactive({
count: 1,
get doubled() {
return state.count * 2
},
increment: () => state.count++
});
return state;
// or
return {
...toBindings(state),
...injected,
...
} |
@jacekkarczmarczyk what problem would that solve though? I feel like I'm missing the point edit: made wording more neutral |
For me it seems more logical to group related things in one object instead of declaring separate variables/methods. Such object could be also used outside of the component scope (including tests) |
We still need to use
People do will excessively use of composition functions, which be equivalent to excessive use of setup() {
const state = reactive({
count: 1,
});
const double = computed(() => state.count * 2)
return {
should I use toBindings, both seem to work. help me... ? state : ...toBindings(state),
double,
};
} |
@yyx990803 Ah, I see what you're saying. Thanks. Alternative proposal then. Instead of |
@tigregalis - I believe in your first example "state" should be "profile" in some places?? Scott |
@smolinari good catch. Thanks for that. I initially started the example with |
@tigregalis Great example 👍 You nailed down exactly why I felt |
Well, Scott |
@smolinari I also see the value of keeping |
It seems a number of us in this thread like @tigregalis have reached the conclusion or at least contemplated that |
I think the biggest advantage of the function-based API is that we can group related things together: const items = binding([]);
const filteredItems = computed(() => ...);
const fetchItems = () => { ... };
const expanded = binding(false);
const expand = () => { expanded.value = true; };
const collapse = () => { expanded.value = false; }; By using const state = reactive({
items: [],
expanded: false
}); This would make it harder to extract item related logic to a separate That's why I think that const user = reactive({ id: 123, name: 'foo' }); But in the template I would refer to it as |
A lot of data lives and breathes in the form of objects. It would be a mess and downright silly to break an object down just to put a Also, if you have general "state" data within your component which needs to be reactive, which might be passed on or used throughout the different logic sections of the component, but not necessarily always related, why not group it up in a single object? That way, you can just use If you ask me, if anything is still an eyesore in this new API? It's needing to add I'm absolutely certain a list of best practices will be needed for code built on this API, because of the flexibility it affords. Like naming functions external to A set of best practices might also be something the Vue team could put together for the Style Guide docs?? 😁 Scott |
I too tend to think binding as being more or less redundant when it is basically the same as reactive with a 'value' property. I was about to write why not initialize a reactive with its .value set depending on wether you pass an object or not.
But then of course you'll get some inconsistencies when you really want to bind an actual Object and not an associative array. In those moments, I find it quite unfortunate that JS does not provide a native Dict-like. |
I think that although I totally understand and respect that you might want to group related data in an object, but for me, rather than take the form (1) const profile = reactive({
visits: 0,
name: 'John Smith',
message: 'Hello'
}) it would be more like (2) const profile = {
visits: binding(0),
name: binding('John Smith'),
message: binding('Hello')
} Perhaps you could import a helper function so that you could write (1) or something similar, and get (2). Maybe that helper is called That would start to look more like the object API, and be a bit of a bridge in understanding between the two: the difference from The question is, though, where would you insert the const profile = {
visits: {
count: 0,
latest: null,
items: []
},
name: 'John Smith',
message: 'Hello'
} |
@tigregalis I personally hate the idea to have to use
And I would only use And even though this is a bit more verbose
it is a small price to pay to do not use It is a shame we cannot just have a compilation step that makes everything inside |
Exactly, I think people will go that way more often than not. return {
state,
}; |
After spending some time taking a closer look at API of frameworks which make use of
So, to me, the question shouldn't be about whether or not we should keep one and ditch the other: both are useful, and both have been available in other frameworks for ages. A more pressing concern I have now is that, we keep drawing a beginner/expert line between
IMHO, this (imaginary) beginner/expert line we draw actually makes the API less cohesive. And with one function being named with a simple term ( Now, regarding the function naming issue, as stated above, cohesive names are important. It doesn't have to be the same as what Mobx does, but the names should at least read/sound like they belong together than this |
I'm coming into this a little late but I think the Aside from the namespacing and it likely being considered "bad practice" by the community, I think that having this function would cause some confusion when it comes to logically grouped data. Consider the case where you actually want to bind an object like so: const user = binding({
name: "jane",
age: 35
}); I believe this is a totally valid use of binding, but if we're talking about "beginner" vs "advanced", it's confusing why you would use this over Introducing a "simple" concept that could almost immediately require a deeper explanation as to when and where it should be used sounds like a step backwards. I'm all for shortening the name of |
@tandroid1 - it's not possible to bind an object with As for namespacing, and spreading objects, how can that be an issue? Only the objects/ variables returned from the Or am I misunderstanding you completely? Scott |
Hey @smolinari - Thanks for the clarification! I definitely misunderstood. For the namespacing, I'm referencing a comment by @tigregalis where they point out that you could do something like this... const visits = reactive({
count: 0,
latest: null
})
const upvotes = reactive({
count: 0,
upvoters: []
})
return {
...toBindings(visits),
...toBindings(upvotes)
} This might not seem like a common case but I would be tempted to do it so that data is grouped logically. That being said, maybe it can be taken care of with documentation. |
From my understanding, the const visits = reactive({
count: 0,
latest: null
})
const upvotes = reactive({
count: 0,
upvoters: []
})
// would be returned as objects, not as bindings.
return {
visits,
upvotes
} Which means, in the template you would be using them as In order to use const state = reactive({
count: 0,
text: '',
foo: '',
bar: []
}) And you use this object throughout your component like so. const double = computed(() => state.count * 2) In other words, as an object, you can use let count = binding(0)
const double = computed(() => count.value * 2) And here you can see, we need to add the Also with bindings, you'd have to name all the bindings singularly in the return object, along with the boilerplate of having to write out all of the setup () {
...
return {
count,
text,
foo,
bar
}
} Whereas with setup () {
...
return {
toBindings(state)
}
} And you'd have I hope that is making sense. 😊 Scott |
I think it should be: setup () {
return {
...toBindings(state)
}
} |
I fully agree that having "magic" names is not the best thing but I am going to suggest this anyway :D I am both inspired by the new SwiftUI thingy and in retaining the
|
Why not just: const state = useState({
count: 1
}); And: const value = useValue(1); Same for computed. Looks for me more inline with hooks and it also does prevent variable name clashing. |
convention: access or tranfer data can only by observable data itself methods. setup() {
const state = reactive({
num: 0,
arr: ['a', 'b', 'c'],
str: 'abc',
})
const double = computed(() => state.count * 2)
function increment() {
state.num++
}
return {
...state.$pick(["num"]) // retains reactivity on mutations made to `state`
double,
increment
}
return {
...state.$pickAll()
}
return {
...state.$omit(["arr"])
}
} |
@o-w-o - Isn't the spread on the I also believe you are missing the greater picture. A Scott |
As a belated note per #63 (comment) I'm a proponent for always returning a The potential issues Evan mentioned were
I'd think these would actually be best practices. If some state is coming from a composition function, IMO it's more readable to always keep it "grouped"/"scoped", so that you always know where a given property is coming from (as opposed to having to reference the destructure statement). You could think of it similarly to following the principle of preferring composition over inheritance. When I posted #63 (comment) , I didn't realize that you could put computeds/methods/etc on reactive/state. This actually resolves a big readability issue I had which I discussed in #42 (comment). Knowing this, I might actually be willing to drop class API for the function API - and I'm wondering if many "object API proponents" would feel the same way (though not all of them, I'm sure). setup() {
const state = reactive({
count: 0,
double: computed(() => state.count * 2),
increment: () => state.count++
});
return state;
} feels significantly simpler and easier to reason about from the perspective of "what's in my state" than setup() {
const state = reactive({
count: 0
})
const double = computed(() => state.count * 2)
function increment() {
state.count++
}
return {
...toBindings(state), // retains reactivity on mutations made to `state`
double,
increment
}
} |
For clarity
My intent with that isn't necessarily "I think returning a reactive should be the only option", but "I think returning a reactive should be the preferred option, and the one used in tutorials, etc" (particularly because it appears to me to be more beginner friendly due to its simplicity). If it's the only option I won't personally complain, but I'm sure others prefer the current proposed approach. |
Can I expect createComponent to take first argument as component name if it is a string? import { createComponent } from 'vue'
export default createComponent('MyComponent', (props: {msg: string}) => {
...
return () => <SomeTSX/>
}) Because I prefer TypeScript-only props, and mainly use TSX / render function, I don't need any other options but setup(), however I still need component name (for devtool debug purpose?) |
Closing in favor of #78 |
This is a proposal for an amendment to RFC #42. I'm posting it here separately because the original thread is too long, and I want to collect feedback before updating the original RFC with this.
Please focus on discussing this amendment only. Opposition against the original RFC is out of scope for this issue.
Motivation
This update aims to address the following issues:
value()
is a concept that objectively increases the learning curve compared to 2.x API.value()
in a single-purpose component can be somewhat verbose, and it's easy to forget.value
without a linter or type system.state()
makes it a bit awkward since it feels natural to writeconst state = ...
then accessing stuff asstate.xxx
.Proposed Changes
1. Rename APIs:
state()
->reactive()
(with additional APIs likeisReactive
andmarkNonReactive
)value()
->binding()
(with additional APIs likeisBinding
andtoBindings
)The internal package is also renamed from
@vue/observer
to@vue/reactivity
. The idea behind the rename is thatreactive()
will be used as the introductory API for creating reactive state, as it aligns more with Vue 2.x current behavior, and doesn't have the annoyances ofbinding()
(previouslyvalue()
).With
reactive()
now being the introductory state API,binding()
is conceptually used as a way to retain reactivity when passing state around (hence the rename). These scenarios include when:computed()
orinject()
, since they may contain primitive values, so a binding must be used to retain reactivity.2. Conventions regarding
reactive
vs.binding
To ease the learning curve, introductory examples will use
reactive
:In the template, the user would have to access the count as
{{ state.count }}
. This makes the template a bit more verbose, but also a bit more explicit. More importantly, this avoids the problem discussed below.One might be tempted to do this (I myself posted a wrong example in the comments):
The spread would disconnect the reactivity, and mutations made to
state
won't trigger re-render. We should warn very explicitly about this in the docs and provide a linter rule for it.One may wonder why
binding
is even needed. It is necessary for the following reasons:computed
andinject
may return primitive values. They must be wrapped with a binding to retain reactivity.It is recommended to return bindings from composition functions in most cases.
toBindings
helperThe
toBindings
helper takes an object created fromreactive()
, and returns a plain object where each top-level property of the original reactive object is converted into a binding. This allows us to spread it in the returned object insetup()
:This obviously hinders the UX, but can be useful when:
The text was updated successfully, but these errors were encountered: