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

[BUGFIX beta] Only pass isComponent with a dash #11222

Merged
merged 1 commit into from
May 19, 2015

Conversation

mixonic
Copy link
Member

@mixonic mixonic commented May 19, 2015

Previously, any key looked up in the container was listed as a component by isComponent. Additionally, the assertion for dashless components was upon lookup in component_lookup. This commit makes isComponent filter for dashes as well as presence in the container.

All paths (like {{dashless}}) in the container returned true from isComponent, which in turn handled keyword redirects and the catch-all keyword behavior. It turn out that, using the Ember-CLI resolver, a
module named app/dashless/template will resolve both template:components/dashless and template:dashless. Thus when using pods, the isComponent check for dashless would pass.

This means if dashless was used as a bound variable in a template, it would be treated like a component and throw an error (for not having a dash).

Additionally, the isComponent check not filtering for dashes means all template keywords (like yield, if, outlet) were creating container lookups. This is far less performant that splitting than scanning the string for a dash.

This commit does not fix all cases. If you name a route with a dash, and use that same dashed name as a variable in a template bad behavior will manifest: The template of that route will render with a generated component backing it. This problem must be resolved when routable components are landed.

Previously, any key looked up in the container was listed as a component
by `isComponent`. Additionally, the assertion for dashless components was
upon lookup in `component_lookup`. This commit makes `isComponent` filter
for dashes as well as presence in the container.

All paths (like `{{dashless}}`) in the container returned true from
`isComponent`, which in turn handled keyword redirects and the catch-all
keyword behavior. It turn out that, using the Ember-CLI resolver, a
module named `app/dashless/template` will resolve both
`template:components/dashless` and `template:dashless`. Thus when using
pods, the `isComponent` check for `dashless` would pass.

This means if `dashless` was used as a bound variable in a template, it
would be treated like a component and throw an error (for not having a
dash).

Additionally, the `isComponent` check not filtering for dashes means all
template keywords (like `yield`, `if`, `outlet`) were creating container
lookups. This is far less performant that splitting than scanning the
string for a dash.

This commit does not fix all cases. If you name a route with a dash, and
use that same dashed name as a variable in a template bad behavior will
manifest: The template of that route will render with a generated
component backing it. This problem must be resolved when routable
components are landed.
@mixonic
Copy link
Member Author

mixonic commented May 19, 2015

Fixes #11214

@mixonic
Copy link
Member Author

mixonic commented May 19, 2015

Sauce is being a punk about IE11. A local check builds fine.

@rwjblue
Copy link
Member

rwjblue commented May 19, 2015

I've restarted it a few times, each time a different browser times out. 🎥 🔛

rwjblue added a commit that referenced this pull request May 19, 2015
[BUGFIX beta] Only pass isComponent with a dash
@rwjblue rwjblue merged commit e09b0ef into emberjs:master May 19, 2015
@rwjblue rwjblue deleted the component-stuff branch May 19, 2015 20:53
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 this pull request may close these issues.

2 participants