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

[2.15.0] {{link-to}} inside a partial stopped working #15621

Closed
kevinansfield opened this Issue Sep 1, 2017 · 25 comments

Comments

Projects
None yet
@kevinansfield
Contributor

kevinansfield commented Sep 1, 2017

After upgrading to 2.15.0 some of our tests started failing because a {{#link-to}} inside of a partial stopped working.

This is the {{link-to}} in question. When rendering there are no immediate errors but the generated href is just # and the data-test-user-id attribute is not output when testing. Clicking the link outputs this warning:

This link-to is in an inactive loading state because at least one of its parameters presently has a null/undefined value, or the provided route name is invalid.

Everything else inside of the partial still renders fine, it's just the link-to that fails.

Changing the partial to a component fixed the problem.

@ivanvanderbyl

This comment has been minimized.

Show comment
Hide comment
@ivanvanderbyl

ivanvanderbyl Sep 3, 2017

Contributor

I've come across a similar problem in 2.15, it seems that a partial rendered inside a yielded block from a component doesn't receive the yielded parameters from that component.

Example:

// -name.hbs
<p>{{name}}</p>

component.js

export default Component.extend({
  name: "Ivan"
})

template.hbs

{{yield name}}

some-route-template.hbs

{{#my-component as |name|}}
  {{partial "name"}}
{{/my-component}}

In this instance, the {{name}} variable in the partial is undefined.

Contributor

ivanvanderbyl commented Sep 3, 2017

I've come across a similar problem in 2.15, it seems that a partial rendered inside a yielded block from a component doesn't receive the yielded parameters from that component.

Example:

// -name.hbs
<p>{{name}}</p>

component.js

export default Component.extend({
  name: "Ivan"
})

template.hbs

{{yield name}}

some-route-template.hbs

{{#my-component as |name|}}
  {{partial "name"}}
{{/my-component}}

In this instance, the {{name}} variable in the partial is undefined.

@steverhoades

This comment has been minimized.

Show comment
Hide comment
@steverhoades

steverhoades Sep 3, 2017

The same thing happens with an #each iteration. The value from #each isn't passed to the context of the partial.

{{#each visibleContent as |record index|}}
  {{partial rowTemplate}}
{{/each}}

Both record and index are undefined in the partial.

steverhoades commented Sep 3, 2017

The same thing happens with an #each iteration. The value from #each isn't passed to the context of the partial.

{{#each visibleContent as |record index|}}
  {{partial rowTemplate}}
{{/each}}

Both record and index are undefined in the partial.

@steverhoades steverhoades referenced this issue Sep 3, 2017

Closed

Addon is unusable with Ember 2.15 #239

10 of 10 tasks complete
@Serabe

This comment has been minimized.

Show comment
Hide comment
@Serabe

Serabe Sep 3, 2017

Member

Reproduction in 2.15. Same twiddle working in 2.14.

Member

Serabe commented Sep 3, 2017

Reproduction in 2.15. Same twiddle working in 2.14.

@CrushedPixel

This comment has been minimized.

Show comment
Hide comment
@CrushedPixel

CrushedPixel Sep 7, 2017

Interestingly enough, I can still reference a "lost" variable's property by its name, e.g. in the rowTemplate I could do record.id but not (get record "id"). Not sure if that's of any help debugging the issue.

CrushedPixel commented Sep 7, 2017

Interestingly enough, I can still reference a "lost" variable's property by its name, e.g. in the rowTemplate I could do record.id but not (get record "id"). Not sure if that's of any help debugging the issue.

@thec0keman

This comment has been minimized.

Show comment
Hide comment
@thec0keman

thec0keman Oct 4, 2017

Is this behavior legacy, or is there an alternative approach that should be considered? We've been trying to use components over partials, but for our largish app there are still many instances where this bug is breaking things. At this point, we are blocked from upgrading, and I am surprised this isn't affecting more folks.

thec0keman commented Oct 4, 2017

Is this behavior legacy, or is there an alternative approach that should be considered? We've been trying to use components over partials, but for our largish app there are still many instances where this bug is breaking things. At this point, we are blocked from upgrading, and I am surprised this isn't affecting more folks.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Oct 5, 2017

Member

No, this isn't legacy behavior per-se, but it is fairly uncommon. You should totally migrate to using components whenever you can (it is massively simpler to reason about), but we should absolutely fix this ASAP.

Member

rwjblue commented Oct 5, 2017

No, this isn't legacy behavior per-se, but it is fairly uncommon. You should totally migrate to using components whenever you can (it is massively simpler to reason about), but we should absolutely fix this ASAP.

@ivanvanderbyl

This comment has been minimized.

Show comment
Hide comment
@ivanvanderbyl

ivanvanderbyl Oct 5, 2017

Contributor

@rwjblue any pointers where I should start looking to help fix this?

Contributor

ivanvanderbyl commented Oct 5, 2017

@rwjblue any pointers where I should start looking to help fix this?

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Oct 5, 2017

Member

I submitted #15674 with some failing tests cases. It seems the issue isn’t as simple as accessing bound property values within the partial (at least it didn’t seem so with my testing). I believe the underlying issue has to do with the way we generate the references used by things like if, unless, with, and get.

Sadly, I don’t have a smoking gun to point at though. :(

Member

rwjblue commented Oct 5, 2017

I submitted #15674 with some failing tests cases. It seems the issue isn’t as simple as accessing bound property values within the partial (at least it didn’t seem so with my testing). I believe the underlying issue has to do with the way we generate the references used by things like if, unless, with, and get.

Sadly, I don’t have a smoking gun to point at though. :(

@kjhangiani

This comment has been minimized.

Show comment
Hide comment
@kjhangiani

kjhangiani Oct 6, 2017

we are also unable to upgrade due to this bug. similar to @thec0keman we have a fairly large ember app which was originally started using EAK, so there's still some lingering legacy functionality that we haven't yet brought to modern ember

we don't have a lot of partials left, but enough that this is a blocker for us until we have the time to migrate / test the remaining partials to components

kjhangiani commented Oct 6, 2017

we are also unable to upgrade due to this bug. similar to @thec0keman we have a fairly large ember app which was originally started using EAK, so there's still some lingering legacy functionality that we haven't yet brought to modern ember

we don't have a lot of partials left, but enough that this is a blocker for us until we have the time to migrate / test the remaining partials to components

@jcbvm

This comment has been minimized.

Show comment
Hide comment
@jcbvm

jcbvm Oct 11, 2017

This one should really get some higher priority, partials are broken since 2.15 and prevent people to upgrade from 2.14 to 2.16.

jcbvm commented Oct 11, 2017

This one should really get some higher priority, partials are broken since 2.15 and prevent people to upgrade from 2.14 to 2.16.

@beatrizdemiguelperez

This comment has been minimized.

Show comment
Hide comment
@beatrizdemiguelperez

beatrizdemiguelperez commented Oct 11, 2017

+1

@ivanvanderbyl

This comment has been minimized.

Show comment
Hide comment
@ivanvanderbyl

ivanvanderbyl Oct 11, 2017

Contributor

I'm going to take a look at this next week after EmberFest.

Contributor

ivanvanderbyl commented Oct 11, 2017

I'm going to take a look at this next week after EmberFest.

@dennis95stumm

This comment has been minimized.

Show comment
Hide comment
@dennis95stumm

dennis95stumm Oct 13, 2017

I've extended the twiddle of @Serabe and found out, that if a property with the same name gets defined in the controller, the partials gets the right value. But if a value in a array is undefined the partial still thinks that it has a truthy value.

dennis95stumm commented Oct 13, 2017

I've extended the twiddle of @Serabe and found out, that if a property with the same name gets defined in the controller, the partials gets the right value. But if a value in a array is undefined the partial still thinks that it has a truthy value.

@dennis95stumm

This comment has been minimized.

Show comment
Hide comment
@dennis95stumm

dennis95stumm Oct 13, 2017

This a bug in Glimmer. See in the referenced issue.

dennis95stumm commented Oct 13, 2017

This a bug in Glimmer. See in the referenced issue.

@dennis95stumm

This comment has been minimized.

Show comment
Hide comment
@dennis95stumm

dennis95stumm Oct 17, 2017

A new glimmer version with the fix was released. After updating the glimmer version this bug should be fixed. I've opened a pull request for updating the glimmer version.

dennis95stumm commented Oct 17, 2017

A new glimmer version with the fix was released. After updating the glimmer version this bug should be fixed. I've opened a pull request for updating the glimmer version.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Oct 17, 2017

Member

FYI - The upgrade to Glimmer 0.29 is a large effort (and unlikely to happen before Ember 3.0.0), @chadhietala backported the fix to the 0.25 branch. We need to release that as a patch version and update to that version instead. I’ll try to get these releases done today...

Member

rwjblue commented Oct 17, 2017

FYI - The upgrade to Glimmer 0.29 is a large effort (and unlikely to happen before Ember 3.0.0), @chadhietala backported the fix to the 0.25 branch. We need to release that as a patch version and update to that version instead. I’ll try to get these releases done today...

@GavinJoyce

This comment has been minimized.

Show comment
Hide comment
@GavinJoyce

GavinJoyce Oct 18, 2017

Member

@rwjblue LMK if there is anything that i can help with the release which includes the patched 0.25 branch (as I'm working on upgrading Intercom at the moment). As far as I can tell, it hasn't yet reached ember canary

Member

GavinJoyce commented Oct 18, 2017

@rwjblue LMK if there is anything that i can help with the release which includes the patched 0.25 branch (as I'm working on upgrading Intercom at the moment). As far as I can tell, it hasn't yet reached ember canary

@jevanlingen

This comment has been minimized.

Show comment
Hide comment
@jevanlingen

jevanlingen Oct 24, 2017

The same problem also exists if using the with helper:

{{#with someProperty as |youNameIt|}}
  {{partial somePartial}}
{{/with}}

Probably the patch will fix this too, but to be sure I do mention it here!

jevanlingen commented Oct 24, 2017

The same problem also exists if using the with helper:

{{#with someProperty as |youNameIt|}}
  {{partial somePartial}}
{{/with}}

Probably the patch will fix this too, but to be sure I do mention it here!

@tehmaestro

This comment has been minimized.

Show comment
Hide comment
@tehmaestro

tehmaestro Oct 26, 2017

Hi. Is there a fix released for this issue? As far as i can tell, most of the helpers are somewhat affected when used in a partial. Is there a workaround?

tehmaestro commented Oct 26, 2017

Hi. Is there a fix released for this issue? As far as i can tell, most of the helpers are somewhat affected when used in a partial. Is there a workaround?

@GavinJoyce

This comment has been minimized.

Show comment
Hide comment
@GavinJoyce

GavinJoyce Oct 29, 2017

Member

Ember 2.16.1 has been released which should fix the issues with partials. Please let me know if you're still seeing issues

Member

GavinJoyce commented Oct 29, 2017

Ember 2.16.1 has been released which should fix the issues with partials. Please let me know if you're still seeing issues

@GavinJoyce

This comment has been minimized.

Show comment
Hide comment
@GavinJoyce

GavinJoyce Oct 31, 2017

Member

While these issues have been fixed, FYI I just found another issue with partials:

#15797

I'll work on fixing this later today

Member

GavinJoyce commented Oct 31, 2017

While these issues have been fixed, FYI I just found another issue with partials:

#15797

I'll work on fixing this later today

@xe21500

This comment has been minimized.

Show comment
Hide comment
@xe21500

xe21500 Nov 2, 2017

Are you guys completely sure it's been solved for 2.15? The twiddle from @Serabe (#15621 (comment)) still fails on 2.15.3, but works on 2.16.1.

Incidentally, I've noticed that if you modify the partial to write the name, it actually prints the correct value...

image

xe21500 commented Nov 2, 2017

Are you guys completely sure it's been solved for 2.15? The twiddle from @Serabe (#15621 (comment)) still fails on 2.15.3, but works on 2.16.1.

Incidentally, I've noticed that if you modify the partial to write the name, it actually prints the correct value...

image

@Serabe

This comment has been minimized.

Show comment
Hide comment
@Serabe

Serabe Nov 2, 2017

Member

It is not. Ember 2.15 is not an LTS, so I don't think a fix will be coming.

Member

Serabe commented Nov 2, 2017

It is not. Ember 2.15 is not an LTS, so I don't think a fix will be coming.

@rwjblue

This comment has been minimized.

Show comment
Hide comment
@rwjblue

rwjblue Nov 2, 2017

Member

Confirm.

Member

rwjblue commented Nov 2, 2017

Confirm.

@xe21500

This comment has been minimized.

Show comment
Hide comment
@xe21500

xe21500 Nov 3, 2017

Oh, ok. I thought the fix being backported to glimmer 0.25 would meant this working on Ember 2.15 too. Guess we'll skip directly to 2.16 then. Thanks!!

xe21500 commented Nov 3, 2017

Oh, ok. I thought the fix being backported to glimmer 0.25 would meant this working on Ember 2.15 too. Guess we'll skip directly to 2.16 then. Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment