Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Add test for template partial rendering with a local variable. #1006

Closed
wants to merge 6 commits into from
Closed

Add test for template partial rendering with a local variable. #1006

wants to merge 6 commits into from

Conversation

ajcollins
Copy link
Contributor

Hi Mark,

After reading the documentation on partial rendering (https://gobuffalo.io/en/docs/partials#local-context) I expected local and global variables to be merged. This doesn't seem to happen at present. That may be the desired behaviour?

This adds a failing test case. It could be combined with the existing test case, depending on your preferred approach.

I've created a candidate fix in plush/compiler.go for which I'll submit an MR for review.

Best wishes,

Alex

@markbates
Copy link
Member

I think the correct fix is here in the buffalo repo, instead of the plush repo, since the partial helper is defined in buffalo. https://github.com/gobuffalo/buffalo/blob/master/render/template.go#L43

@markbates markbates added this to the 0.11.1 milestone Apr 7, 2018
@ajcollins
Copy link
Contributor Author

ajcollins commented Apr 7, 2018

That was what I'd originally hoped, but I couldn't see an elegant way for the partial helper to access the required data: since plush is responsible for binding the arguments to the function call, and already attempts to supply missing args for global context.

How do you feel if the exec call storing a local of the old data and a copy of the current data in the templateRenderer. Calls to partial would then merge supplied data before calling exec? each exec would then restore the old data before returning.

@markbates
Copy link
Member

I think you need to start here https://github.com/gobuffalo/buffalo/blob/master/render/template.go#L29 capture the original Data and hold onto it in the struct. Then in partial you would want to create a new map that merges the renderer Data and the partial's data.

@ajcollins
Copy link
Contributor Author

Snap! I'll do that then - thanks.

…ion into buffalo from plush. Note: whilst this currently tests correctly, in a buffalo app path helper functions do not seem to function correctly.
@ajcollins
Copy link
Contributor Author

Note that whilst this passes the test as written, I'm seeing "missing parameters" errors when using the path helpers in a buffalo app. I'll need to investigate this further.

paganotoni and others added 2 commits April 10, 2018 11:15
* adding clean-obsolete-chunks which will clean up on recompile

* adding dot between css name and hash
@markbates markbates removed this from the 0.11.1 milestone Apr 20, 2018
markbates added a commit that referenced this pull request May 1, 2018
* make sure to merge partial args and the context. fixes #1044. fixes #1006

* adds the environment flag as a build tag when building bins
@markbates markbates closed this May 8, 2018
stanislas-m pushed a commit that referenced this pull request May 12, 2018
* make sure to merge partial args and the context. fixes #1044. fixes #1006

* adds the environment flag as a build tag when building bins
@ajcollins ajcollins deleted the test-partial-with-global-context branch May 14, 2018 20:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants