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] {{link-to}} before application boot #11522

Merged
merged 1 commit into from
Jun 21, 2015

Conversation

tomdale
Copy link
Member

@tomdale tomdale commented Jun 20, 2015

Previously, {{link-to}} would work even if the application had not been booted completely. This behavior is relied on by test harnesses like ember-qunit, since an “integration test” of a component may include a template that uses {{link-to}}. Many people have discovered that they would like to test a component that has a link without booting the entire app.

You wanted a banana but what you got was a gorilla holding the banana and the entire jungle. –Joe Armstrong

During the Glimmer work, we refactored the interface between the LinkComponent and the router into a service. Unfortunately, this caused a regression where rendering the link before the router was completely initialized would raise an exception.

This commit adds a very stupid guard to ensure that we have enough router state to determine if a link is active or not. This whole path could be significantly cleaned up, and in particular, we should move towards using a stubbed routing service in tests that encapsulates all of these concerns.

Fixes #11150.

registry.register('component-lookup:main', ComponentLookup);

let component = Ember.Component.extend({
layout: compile('{{#link-to "about"}}Go to About{{/link-to}}'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wacky double space after link-to

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually @mixonic this is intentional. Many people see code as a dead thing, cold symbols to instruct the unfeeling machine what to do. However, the truth is that code, like any living being, is coursing with energy and its own sense of purpose. Here, I could intuitively feel that the {{link-to}} helper was feeling constrained by being forced into such close proximity with the argument, badly damaging the feng shui of this file.

lol j/k I fixed it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL

@mixonic
Copy link
Member

mixonic commented Jun 20, 2015

lgtm!

// This test is designed to simulate the context of an ember-qunit/ember-test-helpers component integration test,
// so the container is available but it does not boot the entire app
QUnit.test('Using {{link-to}} does not cause an exception if it is rendered before the router has started routing', function(assert) {
Router.map(function(match) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using match AFAICT

@chadhietala
Copy link
Contributor

👍 /cc @nathanhammond

Previously, {{link-to}} would work even if the application had not been
booted completely. This behavior is relied on by test harnesses like
ember-qunit, since an “integration test” of a component may include a
template that uses {{link-to}}. Many people have discovered that they
would like to test a component that has a link without booting the
entire app.

> You wanted a banana but what you got was a gorilla holding the banana
> and the entire jungle. –Joe Armstrong

During the Glimmer work, we refactored the interface between the
LinkComponent and the router into a service. Unfortunately, this
caused a regression where rendering the link before the router was
completely initialized would raise an exception.

This commit adds a very stupid guard to ensure that we have enough
router state to determine if a link is active or not. This whole path
could be significantly cleaned up, and in particular, we should move
towards using a stubbed routing service in tests that encapsulates all
of these concerns.
@tomdale tomdale force-pushed the fix-link-to-regression branch from 378dce8 to c9dbecb Compare June 20, 2015 23:52
rwjblue added a commit that referenced this pull request Jun 21, 2015
[BUGFIX beta] {{link-to}} before application boot
@rwjblue rwjblue merged commit f896a89 into master Jun 21, 2015
@rwjblue rwjblue deleted the fix-link-to-regression branch June 21, 2015 15:30
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.

5 participants