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
Rendering <button> in each loop breaks application #16826
Comments
Yeah, confirmed, we've encountered same error after upgrade from 3.2.2 to 3.3.0 |
I'm somewhat surprised that this broke in 3.3, but I would have expected it to break in 3.4 (due to the angle bracket invocation feature). I'll try to dig in to confirm if it is related... |
Changing to: {{#each buttons as |text|}}
<button>{{text}}</button>
{{/each}} Should resolve the issue (assuming that my guess RE: angle bracket invocation feature is correct). |
@rwjblue yes, changing from |
Just an informational message: I had the same problem with {{#each model as |img|}}. Changing img to i.e. "foto" resolves this error. Is this breaking change (ember-source 3.2.2 => 3.3.0) documented somewhere? |
Just another information: this issue tricks me a lot, the key is: don't use html tag name as the {{#each people as |p|}} {{!-- don't, because `p` is a html tag name --}}
{{p.name}}
{{/each}}
{{#each people as |person|}} {{!-- this is fine --}}
{{person.name}}
{{/each}} I'm not sure if it will be a breaking change, but I hope not. |
@nightire as I can tell, we can use html tag name in |
Confirm, yielding any block param that will be used as an angle bracket invocation will result in this issue. See the following issues for some more background information: |
This change should not have happened during 3.3, and we will try to figure out why it did (and fix it). However, we do intend for it to land as part of 3.4 (along with support for linting in applications as a general guide). |
Should we be expecting for this to be patched in 3.3.1? |
Probably, but its still a good idea to refactor away from the patterns that are currently breaking. The ember-template-lint rule |
I also started getting error: unreachable with this add-on in 3.3... |
Is this considered a breaking change? I'm also a little confused by the RFC as to whether this is allowed (and supposed to work), or allowed at build-time (but breaks at run-time). |
Yes, it is a breaking change and we are working on the fix.
The current behavior matches what the RFC stated (essentially that a block param named The current plan (after discussing with the core team on 2018-07-20) is:
|
I have been having this same issue for the last couple of days as well trying to figure out what was going on:
Changing to this fixed the problem for as well:
Just throwing this out there for more documentation on the issue. |
👍 thanks for that (and sorry for the issue) |
@rwjblue I'm not actually ables to find evidence of the Does it exist somewhere else? |
@cafreeman it's not listed there (though it should be), but the rule is here |
Ah, can't believe I missed that. Thank you! |
The My question is will this have a better error in future? Not everyone will be running the linter and I can imagine coworkers running into this and not knowing what's going on when there error isn't useful. |
Can anyone reproduce this on the latest stable release of Ember? |
@pzuraq yes, I can reproduce this error using 3.8.0 |
Just to make things incredibly confusing, this can happen (it happened to me) {{#each someArray as |item i|}}
{{#if (gt i 0)}}
{{fa-icon item.icon}}
{{/if}}
{{/each}} Even though I wasn't using an Also, this isn't caught by ember-template-lint's |
I can absolutely see how that would be pretty confusing. Internally RE: this not being caught by |
@Techn1x. @rwjblue Icon name cannot be dynamic in |
We have dynamic You can also send in an object which has an |
Sorry - whether or not dynamic icon names work, that's just an issue with the example I gave, but the point still stands. The basis of that particular example (using Also worth noting there's two ember font awesome projects going, depending on the FA version you're using. They probably have differences in what they support etc. |
Is this issue still being looked at? it appears to be nearly a year since it was reported but I encountered the
message today with the following code in Ember v3.4.4 (which I believe is a current LTS version). <select>
{{#each options as |option|}}
<option value={{option.id}}>{{option.name}}</option>
{{/each}}
</select> P.S. I've since changed the code to: <select>
{{#each options as |opt|}}
<option value={{opt.id}}>{{opt.name}}</option>
{{/each}}
</select> which obviously works. |
@Caltor latest LTS - 3.8 https://emberjs.com/releases/ |
@lifeart it also says 3.4 bugfixes until May 2019. |
3.4 will only be receiving security fixes at this point, 3.8 will receive bugfixes until 3.12 is promoted to LTS |
@rwjblue So LTS actually means "we ignore the bug until the LTS period has elapsed"? Interesting. I would have thought bugs spotted within the LTS period would be fixed. Still I understand it's a community project etc etc and it's no big deal to workaround. |
@Caltor #16826 (comment) there is workaround for this case, and workaround pretty clear and it's following official linting rules https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-shadowed-elements.md |
@Caltor - I am sorry that we didn't get this fixed for the LTS. 😩 The main issues here are that it has turned out to be gnarly to fix (I have spent a few hours on it), and in nearly all cases updating to avoid the conflict makes your code better and easier to understand which generally makes fixing this particular bug a bit lower priority than others that are actually blocking folks. |
@rwjblue No worries! If the linter picks it up then that is good. It is only a result of bad coding anyway. |
This issue also happens when a yielded components hash has the same name as helper, and linter doesn't catch that. <AngleTable as |t|>
<t.header />
{{t "unreachable"}}
</AngleTable> |
@BobrImperator I'm not sure that problem is something that is possible to catch ahead of time, and it should have always been a failure because local variables have always taken precedence over globals. The reason we can't catch it ahead of time is similar to why you can't lint against something like this in JS: function foo() {
}
function bar(foo = 'a string') {
foo(); // Error: foo is not a function
} While it's reasonable to assume that this is a shadowing mistake on the developer's part, because of JS's dynamic nature, we can't know for sure that |
I kind of expressed myself badly, I don't really expect this to be caught beforehand. <AngleTable as |action|>
<t.header />
{{action "unreachable"}}
</AngleTable> it could be a different case though |
FYI just spent a couple hours on debugging til I finally googled and found this issue (was using option and the yielded name) so its still out there on 3.15. What was weird was that {{log option}} reported the correct value inside the loop, so never occurred to me the name of the variable could be the problem |
* v3.2.0...v3.4.4 # Conflicts: # .template-lintrc.js # .travis.yml # README.md * Run codemods * Add some missing fa-icons and wrong translator * Do not used reserved keywords in each loops (emberjs/ember.js#16826) * v3.4.4...v3.6.1 * Run codemods * Fix test and update yarn Co-authored-by: Ruben Smit <RubenSmit@users.noreply.github.com>
in ember 3.16.6, it still happen in ios safari. |
I just went through all of the comments once again and I believe the linting rule and the current error message are sufficient to steer the developer in the right direction. Given that it also seems like fixing this issue might open up other wholes in the template compilation, I am closing the issue. |
In ember 3.3.0 following template breaks application:
buttons
is a simple array. In console I can seeUncaught Error: unreachable
. This error does not happen with other tags (i.e. works fine) and does not happen on ember 3.2.2Repository with demonstration: https://github.com/GendelfLugansk/ember-rendering-bug
The text was updated successfully, but these errors were encountered: