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

{{#foreach}}{{else}} fails inside {{#get}} statements with no results #7242

Closed
HenryMarshall opened this issue Aug 22, 2016 · 4 comments · Fixed by #8160
Closed

{{#foreach}}{{else}} fails inside {{#get}} statements with no results #7242

HenryMarshall opened this issue Aug 22, 2016 · 4 comments · Fixed by #8160
Assignees

Comments

@HenryMarshall
Copy link

I'm working on a Ghost Theme and am having trouble getting the foreach helper to respect the else condition (or negation) in conjunction with #get.

Steps to Reproduce

  1. Enable "Public API" in Ghost's "Settings > Labs > Enable Beta Features"
  2. Add the following code to any .hbs page
{{!-- Obviously, this tag should not exist --}}
{{#get filter="tags:does-not-exist"}}

  {{#foreach posts}}
    foo
  {{else}}
    fails to show up
  {{/foreach}}

  {{^foreach posts}}
    fails to show up
  {{/foreach}}

  {{!-- Problem persists using #posts shorthand --}}
  {{#posts}}
    foo
  {{else}}
    fails to show up
  {{/posts}}

  {{^posts}}
    fails to show up
  {{/posts}}

{{/get}}
  1. Check that page in your browser

Expected Result: "fails to show up" appears 4 times (once for each reference).
Observed Result: "fails to show up" never appears

Notes

#get works as expected when the tag does exist. In the above code block, you'd see foo appear twice per post found.

{{#foreach}}...{{else}}...{{/foreach}} works as per the documentation when used outside a #get helper. I had no difficulty reproducing their example:

{{#foreach tags}}
  <a href="{{url}}">{{name}}</a>
{{else}}
  <p>There were no tags...</p>
{{/foreach}}

Technical details:

  • Ghost 0.9.0 and 0.8.0
  • Node v4.4.7
  • Chrome v52
  • sqlite
@kirrg001
Copy link
Contributor

Hey @hbaughman. Sorry to hear you're having trouble.
I assume the else should be part of the {{get}} helper.

{{#get "posts" filter="tags:xyz"}}
  {{#foreach posts}}
    yeah posts
  {{/foreach}}
{{else}}
  no posts found
{{/get}}

@ErisDS
the documentation looks wrong?
https://themes.ghost.org/docs/foreach#section--else-and-negation

@ErisDS
Copy link
Member

ErisDS commented Aug 22, 2016

This is interesting, and one of the reasons why we still consider the get helper a beta feature, we weren't sure if some of the ways it works were best or not and wanted feedback from theme developers, however we've had almost none!

The get helper detects if it has no entries and calls it's own else template, so you would call {{#get something}}.. execute this template {{else}} execute this template{{/get}}

So, in the case where you have wrapped it with the get helper, the {{foreach}} helper will never get to its else condition. The {{#foreach}} docs are not wrong, because if you do execute a foreach, but don't pass it any data, then it would execute its own else template.

I can see that this is potentially confusing, and this behaviour is definitely up for debate. I'm not sure if it is any clearer to not have an else template for the get helper though and it would certainly be weird if we only execute the get helpers else template if there is an error as then you would have the same problem in the case of an error.

Perhaps the best thing here is to update the {{#get}} helper docs to show the usage of {{else}} & how to handle an error. I had thought that documentation was already present, but I've realised it only exists in the cookbook section.

@kirrg001 kirrg001 added the docs label Aug 22, 2016
@HenryMarshall
Copy link
Author

@kirrg001, @ErisDS Thanks so much, that solves it!

Perhaps the best thing here is to update the {{#get}} helper docs to show the usage of {{else}} & how to handle an error.

Once you'd explained how {{#get}} works it makes sense. I'm sure that would have headed off my problem.

To be clear, this is not my current usecase, but I can easily imagine a partial that sometimes gets used within a {{#get}} and other times on a page with [{post}] in context (e.g., index.hbs or tag.hbs).

{{! partials/some_partial }}
{{#foreach posts}}
  {{title}}
{{else}}
  no posts
{{/foreach}}

If you call that from here you'd get the titles or "no posts" as appropriate.

{{! index.hbs }}
{{> some_partial}}

If for some reason you later chose to invoke it from within a {{#get}} the behavior would not be the same unless you replicated the {{else}} condition.

{{! page.hbs }}
{{#get something}}
  {{> some_partial}}
{{else}}
  no posts
{{/get}}

Conceptually, I feel like the partial should function identically in the two different scenarios. For this reason, I think I prefer removing {{else}} from {{#get}}. Going this route would mean that the singular {{#post}} wrapper would also need to support an {{else}} (it may already, though this is not mentioned in the docs). Something like:

{{#get id="1234"}}
  {{#post}}
    {{title}}
  {{else}}
    no post found
  {{/post}}
{{/get}}

we weren't sure if some of the ways it works were best or not and wanted feedback from theme developers

A quick googling didn't discover an RFC for the {{#get}} helper, but if these comments would be more appropriate on some other page, let me know and I can repost them.

@ErisDS ErisDS reopened this Aug 22, 2016
@ErisDS
Copy link
Member

ErisDS commented Aug 25, 2016

Reopened this because, at the bare minimum the docs do need updating. I'm also starting to think that the behaviour should be changed. Mulling it over for the pros and cons 😁

@ErisDS ErisDS added the themes label Sep 19, 2016
@ErisDS ErisDS mentioned this issue Feb 27, 2017
14 tasks
@ErisDS ErisDS self-assigned this Mar 13, 2017
ErisDS added a commit to ErisDS/Ghost that referenced this issue Mar 14, 2017
closes TryGhost#7242

- before this, the get helper's else was used for empty resultsets
- the argument was made that we should fall through to a foreach or with helper's else instead
- I agree that this is the more natural, consistent approach, and so would like to change it for Ghost 1.0

E.g. as of this PR we now have:

{{#get "posts" filter="tag:doesnt-exist"}}
  {{#foreach posts}}
  {{else}}
    this ges executed because there are no results
  {{/foreach}}
{{/get}}

instead of

{{#get "posts" filter="tag:doesnt-exist"}}
  {{#foreach posts}}
  {{else}}
  {{/foreach}}
{{each}}
    this ges executed because there are no results
{{/get}}
kirrg001 pushed a commit that referenced this issue Mar 14, 2017
closes #7242

- before this, the get helper's else was used for empty resultsets
- the argument was made that we should fall through to a foreach or with helper's else instead
- I agree that this is the more natural, consistent approach, and so would like to change it for Ghost 1.0

E.g. as of this PR we now have:

{{#get "posts" filter="tag:doesnt-exist"}}
  {{#foreach posts}}
  {{else}}
    this ges executed because there are no results
  {{/foreach}}
{{/get}}

instead of

{{#get "posts" filter="tag:doesnt-exist"}}
  {{#foreach posts}}
  {{else}}
  {{/foreach}}
{{each}}
    this ges executed because there are no results
{{/get}}
@ErisDS ErisDS mentioned this issue Mar 14, 2017
4 tasks
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 a pull request may close this issue.

3 participants