-
Notifications
You must be signed in to change notification settings - Fork 546
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
Attr fallthrough behavior #26
Conversation
Could we get clarification on how best to reproduce the current class/style attr merging in the 3.x api when these conditions exist:
I think for myself and many others with a lot of JSX experience, we know there are gotchas for exactly where you spread attrs and the desired behavior--spreading before defining your own attrs ensures they aren't over-written, while spreading after has the effect of providing default attr values. |
@alexsasharegan that's a good question. In templates <div v-bind="$attrs" aria-hidden="true"></div> In render functions, Vue will expose a <div { ...this.$attrs } aria-hidden="true"/> Both the template and JSX examples above compiles into: return h('div', mergeData(this.$attrs, { 'aria-hidden': true })) |
Now in 2.x, single bindings always overwrite the |
As the author of vue-functional-data-merge, I like the sound of that! |
This comment has been minimized.
This comment has been minimized.
This doesn't sound so great to me - it's hidden from anybody who hasn't read docs in great detail. I don't think Vue has anything else like it. Isn't there a more declarative way? Adding a modifier or something? [EDIT] or what about a prefix like ! on the attribute |
@Justineo I think that's working normally because in the case of inputs, the If you use a |
@mika76, this kind of stuff gets into the weeds of the underlying abstraction--the virtual dom. Component attributes are described by objects, so when you have to merge attribute objects from multiple sources, the simplest tool is Vue has been really friendly and helpful when it comes to merging attributes, especially given the deep merging of things like I agree that this whole area could benefit from additional detail in the docs. One thing to consider, what assumptions do you have about what would happen currently if you saw a component written with duplicate attributes like this ( <x-button disabled type="button" :disabled="false">
Example Component
</x-button> |
@posva Yeah you are right! I prefer this behavior than relying on the order of attributes in templates. |
As a developer who worked both in vue and react projects, I strongly dislike disabling fall-through for |
Maybe we can get some clarification, @dima74, because my assumption was that class/style merging would continue except when you author your own render functions in function components. I think this does get confusing when you consider there are many ways to describe how a component should render:
And maybe more (are dom/string templates okay for function components?). |
it's the same fall through behaviour except now it has to be explicit. This is to make it consistent with Fragments components (more than one element at the root), which wasn't possible before. You won't need any wrappers |
In class and style bindings that are dependent on a parent/child relationship such as position: relative on a parent and position: absolute on child to place it on a containing unit, this becomes a bit more messy. There are workarounds but I'm concerned that this impedes the ability to position things without some annoyances. Happy to talk through it if anyone would like clarification. |
So to enable the 2.x functionality you need to add I understand the argument that it encourages a more "correct" way of building components. But in the real world people aren't perfect and I'd argue using style/class for child component styling is a great way to achieve your goals in the absence of perfection. I'd be interested to see how many people currently write code like |
I came across one blind spot with this concept: Event modifiers. We currently provide a couple of handy modifiers such as <a @click.prevent="handleLinkClick"> Some of these modifiers (i.e. In Vue 2, if we wanted to listen to a native event and prevent its default behaviour, we could do this: <fancy-a @click.native.prevent="handleLinkClick"> However, this RFC drops support for <fancy-a @click.prevent="handleLinkClick">
<!-- Possible Implementations in FancyA: -->
<a v-bind="$attrs"> <!-- WORKS -->
<!-- OR -->
<a v-on:click.prevent="prepareStuff">
<script>
export default {
methods: {
prepareStuff(e) {
// do something else, then:
this.$emit('click', e.target.value) // BREAKS, argument is not an event object
}
}
}
</script> Edit: Now that I think about it, that shouldn't be a real issue.
Would work even better with something like #16 |
I like the improvements this makes for I use fallthrough classes to handle making adjustments to the layout or positioning of child elements given circumstances that the child components need not be aware of. The documentation attached to this RFCS states the following (emphasis mine):
I think we can all agree on the following rules for the “correct” behavior for component responsibilities and boundaries (regarding styling):
The beauty of Vue 2's behavior is that if a parent component needs to style a child component's root element directly:
With the proposed changes, third-party components would have to allow your parent components to pass a class onto them, which from a component's contractual standpoint is, I suppose, more “correct,” but I really struggle to see any way in which this limitation is an improvement to the developer experience since the API for adding a class to the root element would no longer be consistent.
I really think that the current fallthrough capability for classes and styles is important to maintain. There may be use-cases where a parent component needs to add/remove/adjust a component's default To @casey6 's point, I use fallthrough classes in almost every parent/child relationship, so if this behavior ships in Vue 3, I'm probably going to have to/want to implement an explicit As @dima74 mentioned, having to wire this stuff up manually in React was painful... Vue 3 would still be better than that thanks to class merging, but not better than Vue 2 due to the lack of consistent application. I think that the inconvenience of having to manually wire this up is likely to cause developers to add styles that impact outer-layout directly to their components. Overall, I think that In cases where there are multiple root elements, I would want my fallthrough class applied to each of them. (But if that isn't possible, I'd rather have the inconsistency than the proposed inconveniences, as instances in which I need fragments are fairly rare ( This is one of the bits of functionality that I think makes Vue great, and makes it a cut above the rest. I hope it's possible to keep. |
What about using an options flag, that denotes that the component renders (or could render) more than one root element? It many cases, adding an outer wrapper for styling positioning adjustments can break layout for some CSS libraries that expect a certain element structure, specifically CSS sibling selectors (when components are children of a parent element). How would consumers know if a component will merge style/classes on it's root element? |
After core team discussion I'm closing this since it is a major breaking change for a lot of use cases, and it's very difficult to provide a smooth upgrade path. We will be submitting a new RFC with a smaller scope of changes instead. |
Disable implicit attribute fall-through to child component root element
Remove
inheritAttrs
optionRendered