Help contribute to Ember: non-block form linkTo

1

I’ve been trying to keep an eye out for simpler, lower-priority enhancements to Ember that would be well-suited to someone trying to start contributing or level up, and I think I’ve got a decently straightforward one in mind:

The {{linkTo}} helper presently can only be used in block form, e.g.

{{#linkTo 'about'}}The About page{{/linkTo}}

I’d like for it to be possible to rewrite the above in non-block form, e.g.

{{linkTo 'The About page' 'about'}}

The logic is simple: if a block is NOT provided (i.e. the helper is just invoked as {{linkTo ...}} and not {{#linkTo ...}}block content{{/linkTo}}), the first arg should be treated as the link text.

Also, if the first arg is not a quoted string, it should be treated as a bound path, e.g.

{{#each articles}}
  {{linkTo title 'articles.show' this}}
{{/each}}

In this case, the generic links would use the article title as the link text, and if those article titles changed for any reason, the link text should update.

This seems like an ideal fit for a new contributor; just the right amount of challenge, but more or less localized, and should level up your knowledge of how bindings and helpers etc work in Ember.

Who wants it?

6 Likes
2

It goes without saying that I (and others) will be available to help whoever steps up on their way to getting this done.

3

Hello, @machty. Saw you at the most recent Ember NYC meet-up. I’d be really interested in tackling this one!

4

Go forth! Start with some failing test cases in link_to_test.js and lemme know if you run into any problems.

5

Okay, so I started with this simple failing test case, not entirely sure if it is sufficient to test the functionality/feature we’re working on, I’m not really good at testing ember.js apps, let alone testing ember.js itself, so, what do you think?

Anyway, as you can imagine, the test is failing because handlebars parser is expecting an OPEN_ENDBLOCK:

Died on test #1     at eval (ember/~tests/helpers/link_to_test:681:1)
    at eval (ember/~tests/helpers/link_to_test:698:3)
    at eval (native)
    at http://localhost:9292/minispade.js:17:31
    at Object.minispade.globalEval (http://localhost:9292/minispade.js:18:14)
    at Object.minispade.require (http://localhost:9292/minispade.js:31:20)
    at http://localhost:9292/?module=The%20%7B%7BlinkTo%7D%7D%20helper:210:23: Parse error on line 1:
...t" id="about-link"}}
-----------------------^
Expecting 'CONTENT', 'COMMENT', 'OPEN_BLOCK', 'OPEN_INVERSE', 'OPEN_ENDBLOCK', 'OPEN', 'OPEN_UNESCAPED', 'OPEN_PARTIAL', got 'EOF'
Source: 	
Error: Parse error on line 1:
...t" id="about-link"}}
-----------------------^
Expecting 'CONTENT', 'COMMENT', 'OPEN_BLOCK', 'OPEN_INVERSE', 'OPEN_ENDBLOCK', 'OPEN', 'OPEN_UNESCAPED', 'OPEN_PARTIAL', got 'EOF'
    at Object.parseError (http://localhost:9292/handlebars.js:306:11)
    at Object.parse (http://localhost:9292/handlebars.js:358:22)
    at Object.Handlebars.parse (http://localhost:9292/handlebars.js:676:28)
    at Object.Ember.Handlebars.compile (ember-handlebars-compiler:264:26)
    at Object.eval (ember/~tests/helpers/link_to_test:686:44)
    at Object.Test.run (http://localhost:9292/qunit/qunit.js:190:18)
    at http://localhost:9292/qunit/qunit.js:348:10
    at process (http://localhost:9292/qunit/qunit.js:1420:24)
    at http://localhost:9292/qunit/qunit.js:466:5

I have been fiddling with link_to.js helper trying to figure out how to check whether a block is provided or not, I even checked other helpers that allow for blockless use, but I can’t get my head around the concept used.

I would love to hear your thoughts.

6

The template you’re using has invalid Handlebars code: {{#linkTo "The About Page" "about" id="about-link"}}

Specifically, you’re using a # as if you’re invoking linkTo in its block form, but you’re not providing a block. So, you should get rid of the # and you’ll be good to go.

7

Oh, my bad. I totally overlooked that, I was thinking that it has something to do with how linkTo helper works, not because I provided an invalid handlebars code. The test is failing now because the helper is expecting a route in the first argument, hopefully this is an easy one.

8

That’s the feature you’re implementing :slight_smile:

9

In brief, I did the following:

I started by adding another parameter in the linkTo function helper:

Ember.Handlebars.registerHelper('linkTo', function(text, name) {
});

but then of course, it broke every other test case in the link_to_test.js file, and I realized that it wouldn’t make a lot of sense to be expecting text argument every time linkTo is invoked especially since it is mostly used in block form, so I decided to work with the arguments array directly, and added a check to see if a block is provided or not, like the following:

if(!options.fn) {
  console.log('a block is not used');
}

then I assigned name to the second argument if a block is not provided, as follow:

if(!options.fn) {
  name = arguments[1];
}

so the test failed with a new message:

Error: More context objects were passed than there are dynamic segments for the route: about

After playing with LinkView and exploring how it works internally I realized the hash.parameters object is passed with The About Page and about, both of which are then added to the paths array in LinkView init. So, I thought, maybe we shouldn’t pass The About Page param at all?

I’m not sure I’m understanding how everything works internally, but I have been trying my best to decipher what goes where and so on. Anyway, my next step was trying to remove the first element of the params array inside the linkTo helper so I did the following:

hash.parameters = {
  context: this,
  options: options,
  params: (options.fn) ? params : [].slice.call(params, 1) // checks if a block is given, if not remove the first element of the params array.
};

And, the first assertion passed, href was rendered properly with ‘#/about’ as its value, I even checked it worked properly through the browser, and found that the template renders with the link having its href present (and correct), but the link has no text (obviously):

<a id="about-page" class="ember-view" href="#/about"></a>

I don’t know where to head next (I know the helper should use the first argument as the link text, not sure what to do though), would appreciate if you can help put me in the right direction.

P.S. This gist shows the linkTo helper after the changes I made.

2 Likes
10

yeah, the thing here is how to set the first argument as the template of the view, the documentation for a view says:

{{view}} inserts a new instance of Ember.View into a template passing its options to the Ember.View’s create method and using the supplied block as the view’s own template.

so, it is always expecting a block for the template. Do we need to set a default template when no block is given ? maybe.

11

@ahazem, I did something, but I’m not sure if it is the right thing to do, if you want to make your second tests pass, I defined the block where the linkTo is registered:

if(!options.fn) {
  options.fn = function() {
    return varWhereYouHaveTheText;
  }
}

something like that.

12

@fanta Thanks! That’s exactly what I was looking for, I didn’t know assigning a function to options.fn with a return value of the link text would actually append the text to the link (but now that I think of it, it seems so logical).

So, both tests pass now, however, I’m sure there’s a more elegant solution out there, I will just wait for @machty to weigh in and explain how (if he has enough time and willingness) the inners of LinkView work and to give us some feedback too.

13

@ahazem do you have the latest code up somewhere?

14

@machty [Here][1] you are. [1]: https://gist.github.com/ahazem/6157406

15

@ahazem, don’t forget to run all tests, you might find some of them broken

16

@ahazem Looks good so far. Nice work!

Next up is adding test cases for the bound link title version which will look up the link text on the present context if the arg you provide is unquoted, e.g.

{{#each articles}}
  {{linkTo title 'articles.show' this}}
{{/each}}
17

@fanta Thanks for pointing that out, I ran all tests and the “linkTo should pass jshint” failed because I missed a couple of semi-colons. Now fixed, all passes.

18

Well, I wrote this test case, it looks up the index controller and then sets articles array into the controller, then after booting the application and navigating to the root, we lookup the titles of the articles using mapProperty and test against them. The last two assertions ensure that href is rendered correctly.

Switching focus back again to the helper, I added the following code:

if(options.types[0] === "ID") {
  linkText = get(this, linkText);
}

that check to see if the first argument type is an ID, not a string (in which case first argument would be unquoted), afterwards we lookup the current context ‘this’ to get the value of first argument. Now, the first two assertions pass, however, the other two fail (instead of having /articles/1 it returns /articles/ for instance) but I can’t figure out why.

@machty could you please have a look at the test case and my link_to.js file found here (I know that the helper function needs some DRYing) and give me some feedback? :slight_smile:

19

I’m gonna be mega busy today and tomorrow before heading out on a trip, so I’m hoping someone will be able to step up and advise if I’m slow going, but I’ll try to take a look shortly.

20

Okay, so today I had the time to set down and have a closer look at the internals of link_to.js and LinkView. As it turns out, the aforementioned two assertions are failing because Ember.Handlebars.resolveParams goes through each param passed and if the param type is ID it is looked up using current context (if it’s a STRING, the param is returned as is).

Now, if we have the following code:

{{#each articles}}
  {{linkTo title 'articles.show' this}}
{{/each}}

we could easily check to see if the second argument is a STRING and then change it to ID, but this wouldn’t be very thoughtful, would it? I have to admit I’m not getting anywhere with this.