🐛 Fixed async helpers nested in template helpers

[allouis]

closes #10643

The async resolver in express-hbs relies on storing the state of the
promises on the this value inside of a helper, which is always set to
the context. This patch updates our helpers which render templates, to
use this as the context when rendering their templates.

[allouis]

This PR updates the navigation and pagination helpers to forward the context from the helper to the underlying template. So it does technically change the functionality, both by providing more context to the sub template, and modifying the context at the level of the helper.

I'm wondering if the current way was intentional, and this change would be a regression @ErisDS ?

1. The reason we make the change is because express-hbs stores a "resolverCache" to look up async values from.

2. The cache is created at the top level and stored on this or context to be passed down to nested templates.

   2.a. It will never be stored on options AFAICT because always one of context or this will exist_

3. When we render the templates, we don't forward the context or the this value.

4. The sub template appends promises to be resolves to a resolverCache that the top level has no idea about.

If this is a regression, we can fix it in express-hbs, but either way, the implementation for async helpers in express-hbs is pretty brittle, and I think needs a refresh to stop something like this cropping up again.

[ErisDS]

Nothing intentional, but IMO using .merge/.extends to override the first variable AND assigning is an antipattern due as it looks like the first variable has been unexpectedly overridden. IMO should be one or the other.

[allouis]

@ErisDS I've updated to simply assign to this rather than _.merge and assign to new identifier.

[ErisDS]

I've also remove the assigning options.hash because both helpers are just

used without arguments or hash e.g. {{navigation}} {{pagination}} This is incorrect. We want to be able to pass variables to these templates, if you look up my previous commit on these helpers you’ll find history of this change with the relevant issue.

…

On Mon, 10 Jun 2019 at 07:47, allouis ***@***.***> wrote: @ErisDS <https://github.com/ErisDS> I've updated to simply assign to this rather than _.merge and assign to new identifier. I've also remove the assigning options.hash because both helpers are just used without arguments or hash e.g. {{navigation}} {{pagination}} — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10750?email_source=notifications&email_token=AAAYZCPI4LDCKGXTWSA3ZDLPZX2JLA5CNFSM4HOAVLF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXJCEQA#issuecomment-500310592>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAYZCJTGASW7MCETYVU46LPZX2JLANCNFSM4HOAVLFQ> .

[allouis]

@ErisDS Rebased to reinstate this functionality