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

[Glimmer] Nested components no longer have access to their parent? #11170

Closed
omghax opened this issue May 14, 2015 · 15 comments · Fixed by #11266
Closed

[Glimmer] Nested components no longer have access to their parent? #11170

omghax opened this issue May 14, 2015 · 15 comments · Fixed by #11266
Milestone

Comments

@omghax
Copy link
Contributor

omghax commented May 14, 2015

I recently tried upgrading ivy-tabs to Ember 1.13 and ran into some issues around changes to parentView in components. Prior to 1.13, a nested component could access its outer component through parentView. I was using this in ivy-tabs to register an ivy-tab component with the ivy-tab-list that contained it, among other things:

{{#ivy-tab-list}}
  {{#ivy-tab}}Tab 1{{/ivy-tab}}
  {{#ivy-tab}}Tab 2{{/ivy-tab}}
  ...
{{/ivy-tab-list}}

It seems that with the 1.13 upgrade, parentView is now always the "routable" view that rendered the component, and I don't see a way for components to access each other at all. Is my example an intended use case? If not, is there some other way to do what I want?

@ZebraFlesh
Copy link

I'm seeing the same thing as well in ember-cli-happy-splitter. Among other uses, I was using the parentView to manually set the target (so Ember.Component.send() sends actions to the right place) and to create computed properties (Ember.computed.readonly('parentView.someParentProperty').

My structure is similar, but I also support arbitrary levels of nesting:

{{#happy-split-container}}
  {{#happy-split-view}}<p>left panel</p>{{/happy-split-view}}
  {{~ happy-splitter-bar ~}}
  {{#happy-split-view}}

    {{#happy-split-container isVertical=false}}
      {{#happy-split-view}}<p>top right panel</p>{{/happy-split-view}}
      {{~ happy-splitter-bar ~}}
      {{#happy-split-view}}<p>bottom right panel</p>{{/happy-split-view}}
    {{/happy-split-container}}

  {{/happy-split-view}}
 {{/happy-split-container}}

@mixonic
Copy link
Member

mixonic commented May 15, 2015

@wycats it seems maybe the definition of parentView has changed to mean the view managing the light dom of a component?

@Kerrick
Copy link
Contributor

Kerrick commented May 15, 2015

This also seems to affect the use of nearestOfType. See Ember 1.13.1-beta.1 vs Ember 1.12.0

@mike-north
Copy link
Contributor

nearestOfType, nearestWithProperty, and parentView all seem to have changed. This is a common pattern to follow for child components pulling data from (or pushing data to) a wrapper of some sort.

mike-north/ember-cli-materialize#113

@mixonic mixonic added this to the 1.13.0 milestone May 19, 2015
@shripathee
Copy link

Seeing this in Canary. This is potentially a very big change, as nested components are very commonly used.

@stefanpenner
Copy link
Member

This seems to break a large number of addons

@mike-north
Copy link
Contributor

Unfortunately this is preventing me from doing much in-depth testing of the 1.13-beta.1 branch

@rwjblue
Copy link
Member

rwjblue commented May 21, 2015

Given the relatively large number of addons that are affected by this, I think we should attempt to match the 1.12 semantics with this.get('parentView') from within a component.

Given the initial example:

{{#ivy-tab-list}}
  {{#ivy-tab}}Tab 1{{/ivy-tab}}
  {{#ivy-tab}}Tab 2{{/ivy-tab}}
  ...
{{/ivy-tab-list}}

Inside the {{ivy-tab}} instances, calling this.get('parentView') should return the instance of {{ivy-tab-list}}.


Even though we should definitely fix this issue, I do believe that the better pattern would be to use block params and provide each {{ivy-tab}} instance with its parent. So the above example would become:

{{#ivy-tab-list as |list|}}
  {{#ivy-tab list=list}}Tab 1{{/ivy-tab}}
  {{#ivy-tab list=list}}Tab 2{{/ivy-tab}}
{{/ivy-tab-list}}

Yes, I realize that may not be as aesthetically pleasing but it is easier to reason about. In the long run, I think the following is the best API:

{{#ivy-tab-list as |list|}}
  {{#list.ivy-tab}}Tab 1{{/list.ivy-tab}}
  {{#list.ivy-tab}}Tab 2{{/list.ivy-tab}}
{{/ivy-tab-list}}

It allows the ivy-tab child component to be completely scoped to its parent (without the manual wiring of list=list), and is still quite declarative. This syntax will be unlocked by #10244 once I have time to work on it again.

tldr; I think we should deprecate accessing parentView and suggest using block params.

@mike-north
Copy link
Contributor

👍 @rwjblue - This would bring semantics closely in line with ruby conventions like

= form_for 'user' do |f|
  = f.input 'email'

Does this mean that nearestOfType and nearestWithProperty will be deprecated as well?

@rwjblue
Copy link
Member

rwjblue commented May 21, 2015

@truenorth - I am unsure, I will have to confer with the smart core team members :trollface:

@stefanpenner
Copy link
Member

Does this mean that nearestOfType and nearestWithProperty will be deprecated as well?

i would like to explore this

@rwjblue
Copy link
Member

rwjblue commented May 24, 2015

#11266 should fix this issue. We decided not to deprecate accessing parentView at the moment because it does not make much sense to force component authors to switch to an alternative now only to switch to scoped helpers based solution when that lands (hopefully in an early 2.x release).

@tpitale
Copy link
Contributor

tpitale commented Jul 10, 2015

Can we reopen this? https://github.com/emberjs/ember.js/blob/master/packages/ember-htmlbars/lib/hooks/component.js#L25 seems to have lost the changes by @rwjblue.

Or, is there another syntax to be using now?

@spruce
Copy link

spruce commented Jul 24, 2015

👍

@chriscareycode
Copy link

Here is a shim I threw together for nearestWithProperty that works with Ember 2.0.0

nearestWithProperty: function(prop) {
    var parent = this;
    while(true) {
      parent = parent.get('parentView');
      if (typeof parent === 'undefined' || !parent) { return; }
      if (parent[prop]) { return parent; }
    }
  }

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.