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.

Sign up for GitHub

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

Jump to bottom

Re-rendering a template using {% embed %} fails #767

Closed
barnslig opened this issue Jan 5, 2021 · 4 comments · Fixed by #837
Closed

Re-rendering a template using {% embed %} fails #767

barnslig opened this issue Jan 5, 2021 · 4 comments · Fixed by #837
Assignees
@willrowe
Labels
pending - needs review priority - high triaged type - bug / bugfix
Milestone
v1.16.0

Comments

@barnslig
Copy link

barnslig commented Jan 5, 2021

Problem description

When using {% embed %}, re-rendering the template causes a Cannot extend an inline template error.

allowInlineIncludes is enabled.

Expected behaviour

The render function can be called multiple times to re-render the template.

Detailed bug description and possible fixes

The bug is caused by assigning the resolved template as parentTemplate on the first render: twig.core.js, line 1360.

The second time the template is rendered, template.parentTemplate is already the assigned template object and could directly be used. However, Twig.Templates.load() is called again, this time with the object instead of the template path, returning null: twig.core.js, line 1340. This results in an empty parentTemplate and thus a failing render.

I found two possible fixes:

  1. Remove the assignment in line 1360
  2. Introduce a check in line 1340 whether template.parentTemplate is already the resolved object

Is there any preference on one of the solutions? I am not sure which impact either of the solutions have.

Reproducing the bug

I've created an example repository to reproduce the bug: barnslig/twigjs-embed-rerender-bug
@YoungElPaso
Copy link

I have a similar experience trying to extend in an inline template. Same error I believe.

@norin89
Copy link

norin89 commented Apr 9, 2021

I have exactly the same issue.

@pjoulot
Copy link

pjoulot commented Aug 23, 2021

I had a similar experience using an include. Same error as well.

@willrowe
Copy link
Collaborator

I have confirmed this is a bug.
Here is a TwigFiddle that demonstrates how it works in TwigPHP.
Here is what happens with twig.js: https://codepen.io/willrowe/pen/abqrZXW

@willrowe willrowe added this to the v1.16.0 milestone Jun 20, 2022
@willrowe willrowe pinned this issue Jun 20, 2022
@willrowe willrowe self-assigned this Sep 22, 2022
willrowe added a commit to willrowe/twig.js that referenced this issue Oct 3, 2022
@willrowe
Add test for issue twigjs#767
e35788f
@willrowe willrowe mentioned this issue Oct 3, 2022
Merged
willrowe added a commit that referenced this issue Oct 5, 2022
@willrowe
Merge pull request #837 from willrowe/test/issue-767
22e7a38 
Add test for issue #767
@willrowe willrowe unpinned this issue Jan 3, 2023
RobLoach added a commit that referenced this issue Feb 28, 2023
@RobLoach
v1.16.0
8d69d0a 
Major improvements:
* Fix passing context around by @willrowe in #850
* Add namespace support to `source` function by @willrowe in #823
* Use src/twig.js as package's main script instead of the compiled twig.js by @RobLoach in #829

Minor improvements:
* Fix macro changing context in loop by @mihkeleidast in #773
* Imported function PATHS.strip_slash() missing by @murageyun in #770
* Convert non-string values to string before replacing by @kmonahan in #797
* Add GitHub actions test workflow by @willrowe in #817
* Fix date parsing with timezones by @plepe in #765
* Fixed Twig official's URL on README.md by @Geolim4 in #822
* Add tests for whitespace in paths by @willrowe in #824
* Fix multiple includes with embeds by @willrowe in #828
* Update to Mocha 9.x by @RobLoach in #831
* Add test for issue #767 by @willrowe in #837
* Add support for `divisible by` test by @willrowe in #838
* Add support for `with` tag without context or `only` keyword by @willrowe in #839
* Use v3 of `actions/checkout` by @willrowe in #846
* Test on more node versions by @willrowe in #847
* Fix webpack 5 compatibility by @willrowe in #849
* Add test to confirm `renderFile` error handling by @willrowe in #851
* Fix casing of variables in docs by @willrowe in #852
* Bumped dependencies by @dependabot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@willrowe willrowe

Labels
pending - needs review priority - high triaged type - bug / bugfix
Projects
None yet
Milestone
v1.16.0
Development

Successfully merging a pull request may close this issue.

Add test for issue #767 willrowe/twig.js
5 participants
@YoungElPaso @barnslig @pjoulot @willrowe @norin89