Skip to content

Document removal of $children API #454

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

Closed
bencodezen opened this issue Sep 4, 2020 · 8 comments · Fixed by #670
Closed

Document removal of $children API #454

bencodezen opened this issue Sep 4, 2020 · 8 comments · Fixed by #670
Assignees

Comments

@bencodezen
Copy link
Member

Per @posva:

I think it's worth documenting the removal of $parent (and $children) as well vuejs/core#2044 (comment)

@rowanwins
Copy link

+1 , I've run into the lack of $children in porting ViewUI component library and haven't found any advice yet as to how I ought to replace it.

@LinusBorg LinusBorg self-assigned this Oct 16, 2020
@LinusBorg LinusBorg changed the title Document $parent and $child API change Document removal of $parent and $chilren API Oct 16, 2020
@LinusBorg LinusBorg changed the title Document removal of $parent and $chilren API Document removal of $parent and $children API Oct 16, 2020
@LinusBorg
Copy link
Member

LinusBorg commented Oct 16, 2020

@rowanwins in what way do you use $children that is hard / can'T be done with a ref in an ergonomic way?

I'll document the removal and would like to add guidance for use cases that aren't easily solved with refs.

@tim-janik
Copy link

@rowanwins in what way do you use $children that is hard / can'T be done with a ref in an ergonomic way?

Here are a few examples I'm running into during my Vue2->Vue3 porting work:

  • I have components like menubar, contextmenu, menuitem, submenu, modal dialog, tree selection (like a file tree where items can be selected as part of a popup menu or modal dialog).
  • In a contextmenu popup scenario, I'm walking $children recursively to invoke something like a check_active function that evaluates application state to determine if a menu item needs to be inactive. Doing this any other way would be prohibitively expensive in code complexity.
  • Some popups have hotkeys configured on the menu items or tree items, I'm walking $children recursively to construct a hotkey map that can be temporarily active e.g. during modal popups, or in the case of a menubar dropdown permanently (think Ctrl+O -> Open, Ctrl+S -> Save, Ctrl+Q -> Quit, etc).

Basically, Vue allows creating components by composing other elements/components in a nested fashion. Recursively walking those elements is a very natural way of programming a large variety of algorithms. That's why all comparable toolkits allow walking of children, e.g. Gtk+, the DOM, Qt, etc.
Some uses can be implemented differently, e.g. by contextmenu/menubar providing hooks for hotkey registrations in menuitems received via inject, but that gets ugly for e.g. tree items that may or may not be embedded in a menu and doesn't scale to future algorithms needing recursive walks.
So its often cleaner to define a particular interface on components, e.g. an extract_activation_hotkey() function, and then walk subtrees recursively, while handling components that follow the interface and special DOM elements, while ignoring others.

That said, for my uses, I'm not going to complicate my component implementations further [*], I rather "fix" my recursive walk to deal with yet another Vue3 oddity, like resorting to DOM walks plus extracting __vnode or similar.

[*] I'm starting to regret engaging in the port, I've had to add lots of boilerplate to my Vue2 components, because of v-model incompatibilities, emits (just a pain if you've been satisfied with Vue2 here), v-slot incompatibility, v-for keys, and the most nasty one: replace all
component?.maybefoo accesses with
Object.hasOwnProperty.call (component, 'maybefoo') && component.maybefoo
just to avoid Proxy warnings that have been largely useless for me.

Let me know when you have a mailing list to discuss these kind of "war stories" and want to hear more about a user perspective in porting ;-)

@rowanwins
Copy link

rowanwins commented Oct 19, 2020

Thanks for chiming in @LinusBorg , even knowing that using ref's would be the suggested approach is helpful 👍

In the codebase I'm looking at there are a whole bunch of scenarios, a couple for example

Moving to refs seems feasible and I don't think it would add too much complexity. Also I'm not the owner/maintainer of the lib so I won't argue whether this was ever the most optimal vue-esque way of doing things, so I may do some other refactoring of that code also.

So far most of my migration has gone reasonably well, I've found the migration docs pretty good so far so thank you!

@tim-janik
Copy link

If the doc writers concede that cases exist that do require $children, here is an emulating mixin I'm using now. Turns out that vue-3.0.0 still provides $parent, but it's inaccurate (null) for components nested inside transitions. So I had to resort to provide+inject on every component (yuk!).
That however produces a Vue-warning about not finding an injection for the toplevel - and it cannot be hacked around, because inject cannot be a function, while provide can.

/// Provide `$children` (and `$vue_parent`) on every component.
const mixin_$children = {
  provide() { return { '$vue_parent': this }; },
  inject: [ '$vue_parent' ],    // warns on toplevel component
  beforeCreate () {
    console.assert (this.$children === undefined);
    this.$children = [];
  },
  created() {
    if (this.$vue_parent)       // using $parent breaks for transitions
      this.$vue_parent.$children.push (this);
  },
  unmounted() {
    if (!this.$vue_parent)
      return;
    const pos = this.$vue_parent.$children.indexOf (this);
    if (pos < 0)
      throw Error ("failed to locate this in $vue_parent.$children:", this);
    this.$vue_parent.$children.splice (pos, 1);
  },
};

@LinusBorg
Copy link
Member

Turns out that vue-3.0.0 still provides $parent, but it's inaccurate (null) for components nested inside transitions.

Seems like a bug in itself.

@LinusBorg
Copy link
Member

LinusBorg commented Oct 19, 2020

Tipp for your mixin:

inject: {
  $vue_parent: {
    from: '$vue_parent',
    default: null,
  }
},

@LinusBorg
Copy link
Member

LinusBorg commented Oct 19, 2020

@tim-janik I'd be interested both in a conversation about your migration pain (some of the ones you listed are a bit of suprise, others not so much) as well as one about the details of your implementation of e.g. the tree selector.

For the former, I'm afraid we don't have a mailing list and likely won't start one (I personally would be hard pressed to say where one would do that), but I'd be available on Discord (chat.vuejs.org).

Concerning the latter: I don't doubt that you get something out of $children that you can't get with sufficent ease from other APis. I'd like to get a better understanding why that is and how we can approve in that area.

You see, I get that it's convenient to access components this way and imperatively control them. It doesn't fit very well into the rest of the self-sufficient / encapsulated component philosophy though, and I'd like to get a better understanding where members of the community feel the need to fall back to such APIs.

I have built stuff like trees and dropdowns, modals etc - but have usually not felt a need to use $children - which doesn't negate your experience, I just wanna broaden mine. So it would be great if you could provide a more specific example, with code if possible.


Sidenote: I tried to replicate the problem with <transition> but failed:

https://codesandbox.io/s/shy-microservice-yxw0j

I get a component reference for $parent no matter where I call it. The only thing to work around is that when in a transition, we have to walk the chain further up.

@LinusBorg LinusBorg changed the title Document removal of $parent and $children API Document removal of $children API Oct 19, 2020
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 a pull request may close this issue.

4 participants