Upcoming linkTo changes/ideas

Proposals
1

Hallo.

linkTo will likely be getting some enhancements shortly. Just wanted to get people’s thoughts / potential objections on them

Unquoted route arg will perform a property lookup

Presently interchangeable:

{{#linkTo foo}}Foo{{/linkTo}}
{{#linkTo 'foo'}}Foo{{/linkTo}}

But shortly, the first one will perform a property lookup for the foo property on the present template context. So if your controller defines foo: 'index', then the linkTo will generate a link to index. If foo changes, the link will be updated.

This will certainly break many apps, but even though it’s unfortunately made its way into documentation (I’m to blame in some cases), quoteless parameters were never intended to be wholesale treated like strings; it’s an accident of somewhat undefined behavior that they’ve been treated this way for this long. So this applies to things like {{action foo}} as well, though that may be addressed in a PR separate from the one that facelifts linkTo.

So, how should we proceed? We could:

  1. Do nothing. Just make the change, publish an RC blog post explaining what’s up
  2. Make the new behavior opt-in via a temporary flag.
  3. Make the new behavior opt-out via a temporary flag.
  4. Something else?

String/Number params for linkTo / transitionTo treated as URL params

Presently, both linkTo 'routeName' foo bar and transitionTo('routeName', foo, bar) will supply contexts to the transition to ‘routeName’ for the leaf-most dynamic routes, but this requires having an object for the route you’d like to transition to. A major missing use case is when all you have is the id, and you’d like to use the destination route’s model hook with that id to perform the transition.

I propose that if the contexts you provide is a number or a string, then it’s assumed to be parameters for dynamic segments, e.g. you could transition to ‘/posts/:post_id’ via transitionTo('posts.show', 5) or transitionTo('posts.show', '5'). This would mean that you can’t just use a string or a number as a route’s model, but I don’t anyone really does that anyway, and even if they did it’d be trivial to just wrap it in an object.

Thoughts on all the above?

5 Likes
2

I think having the new linkTo come with an opt-in flag is best, since that will keep the community from going haywire, and would still allow those who understand to make the changes, and give new individuals the ability to start in that direction. This would have to be documented very clearly.

And after some time, the flag could be removed and the change permanent (1.0.0?).

Another option would be to check if foo is defined on the context, and if not, interpret it as the path, and then nothing would break and everyone is happy. Although, that would make a transition more difficult from the {{linkTo foo}} to {{linkTo 'foo'}}, since both work and there is no flag. So everyone would just keep doing what they were doing, and then the deprecated feature is removed and everything breaks…

Wow, I can’t tell you how much of a headache it is to setup programmatic transitions, when you have to duplicate model queries. This sounds great, especially since before I knew that I had to pass in a model, I assumed I was supposed to pass in the query parameter, obviously this didn’t work… It would be so much more natural.

Totally for it!

3

I agree that an opt in flag makes most sense. I’d be tempted to print a message to the console in dev mode if they flag hasn’t been set to make people aware of it, much like was done when computed properties became cached by default a while back.

I’m less sure about the second part - it does seem to make sense but I’m not sure about not being able to use a raw string or number as a model, are there really no cases where that is necessary? Perhaps not.

4

An absolute yes to your first suggestion, as I’ve had to use programmatic collection views and list item views in order to achieve dynamic t-bootstrap navigations and breadcrumbs, for example. It should have worked this way from the beginning, so I’m inclined to say we should be aggressive with the change (option #1).

However, to avoid breaking changes I guess an opt-in feature flag is fine for now, as long as it’s paired with a deprecation warning.

Regarding the second suggestion, I agree with you proposal. I, personally, would never consider js primitives to be models (though angular doesn’t draw that line), so I don’t see a potential conflict. In an older RC I remember you couldn’t use an object controller as the param for linkTo (which of course was silly), and I see this in a similar vein, simply expanding the usefulness of the helper so users can do what they expect. As long as there is no conflict/overlap, the more we can extend its functionality the more users can offload their code onto hb templates.

5

I’m leaning toward opt-in flag + deprecation warning for {{linkTo unquoted}}

3 Likes
6

Totally agree that opt-in flag is the way to go.

7

Definitely should be opt-in with deprecation, especially since looking at my code base I haven’t quoted any of my {{#linkTo}}. Are there any other helpers where we have this issue?

8

PR: https://github.com/emberjs/ember.js/pull/2905

9

For what it’s worth, one use-case that motivates this change is the ability to create components that include links:

<!-- components/nav-item -->
{{#linkTo route tagName="li"}}<a href="#">{{yield}}</a>{{/linkTo}}

And usage:

{{#nav-item route="articles"}}Articles{{/nav-item}}

It’s also useful for looping over a list of routes and making links:

<ul>
{{#each breadcrumb in breadcrumbs}}
<li>{{#linkTo breadcrumb.route}}{{breadcrumb.description}}{{/linkTo}}</li>
{{/each}}
</ul>

The ability to do this kind of abstraction is important enough to fix this inconsistency ASAP.

2 Likes
10

A deprecation warning would be good. Also, I like the idea of passing multiple arguments as the params for the routes. I think it would be perfectly acceptable to pass the arguments as is to transitionTo, whether they be strings or numbers but if you pass an object would the link update? ie. {{linkTo 'posts.comments.show', post, comment}}

11

I agree with you.

There is already some mis-information about what @machty refers to as undefined behaviour, the opt-in flag will only perpetuate and add to the confusion, in my view.

If you break it, and you break it hard, you’ll force developers to notice it, and hopefully learn it right :smile:

12

It was merged in earlier today with some pretty noisy/obvious deprecation warnings if you use linkTo without quotes and haven’t enabled the flag. I think this will be palatable for most people but if more people on this thread feel strong in favor of the hard break, maybe we can update it.

13

Here’s the PR for the second part of the proposal https://github.com/emberjs/ember.js/pull/2913

14

@machty am I right in assuming that people who DON’T opt-in are really only buying themselves a little time to make the changes because the 1.0 Final will have the Hard Break in it?

15

That is correct. Perhaps the hard break will be in RC8 or something, assuming the numbers go that high.

16

Here’s a new PR that may interest people: https://github.com/emberjs/ember.js/pull/2942

This is merge of @drogus’ and my work. It makes linkTo fully dynamic, and adds loading classes to link elements if the provided contexts are null/undefined.

Here’s an example JSBin: http://jsbin.com/ucanam/262/edit

17

One last improvement I’m considering is allowing the use of {{else}} in {{linkTo}} that, if provided, will display the else block while the linkTo is not loaded, according to the above PR’s description of what loading means. So basically something like

{{#linkTo dynRoute currentObject}}{{currentObject.name}}{{else}}Loading...{{/linkTo}

Nice in theory, but possibly overkill. Is anyone missing this in their current apps?

18

@machty I’m using the latest {{#linkTo}} stuff to be able to pass ids instead of models. The model hook returns a promise. When I load the route through the URL I hit the LoadingRoute but when using the {{#linkTo}} helper it transitions without putting up the loading template.

Expected behavior? I am I doing something wrong? A bug?

Thanks for all the awesome work. I’m really liking the async router stuff.

19

@raytiley Not sure what all you’re doing in your app and what persistence lib you’re using, but it sounds like by the time you click a linkTo, the target model has already been loaded into the identity map, so the promise resolves immediately. If a promise resolves immediately (due to the way RSVP promises in Ember resolve within the run loop), it won’t throw up the LoadingRoute template.

But if you just refresh the page the first time, there’s no earlier point at which the record could have been loaded, so it’ll take a little while for the promise to resolve, in which time you’ll see the loading template.

Seem consistent with what’s happening in your app?

20

@machty unfortunately not. I’m actually loading a bunch of stuff in the model hook and returning a group promise using Ember.RSVP.all(). A few of the promises are Ember-Model findQuerys which hit the server every time. If I have the network inspector open I see the request pending while still on the original route. Once all the network requests finish the UI switches to the new route, with no Loading route ever appearing.

Should I maybe be doing the findQuerys in an Ember.run? I’m wondering if the UI thread is blocked until everything has loaded.