Skip to content

Emit TemplateParsed events for templates loaded by render and include statements - #873

Merged
sebastienros merged 5 commits into
mainfrom
copilot/emit-template-parsed-events
Nov 20, 2025
Merged

Emit TemplateParsed events for templates loaded by render and include statements#873
sebastienros merged 5 commits into
mainfrom
copilot/emit-template-parsed-events

Conversation

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown
Contributor

The TemplateParsed callback was only invoked for top-level templates in FluidViewEngine, but not for nested templates loaded via {% render %} and {% include %} statements in the core library. This prevented AST visitors from transforming nested templates.

Example:

{# root.liquid #}
{{ 1 | plus: 2 }}
{% render 'inner' %}

{# inner.liquid #}
{{ 2 | plus: 2 }}

Previously, applying a visitor via TemplateParsed would only transform root.liquid (output: 5\n4), not inner.liquid. Now both are transformed (output: 5\n8).

Changes

  • TemplateOptions: Added TemplateParsedDelegate and TemplateParsed property
  • RenderStatement & IncludeStatement: Invoke TemplateParsed callback when templates are first parsed, before caching
  • FluidViewEngineOptions: Removed duplicate TemplateParsedDelegate and TemplateParsed property - now uses TemplateOptions.TemplateParsed instead
  • FluidViewRenderer: Updated to use TemplateOptions.TemplateParsed via the options property
  • Tests: Added coverage verifying callback is invoked only once (on first parse), and modified templates are cached; updated ViewEngine tests to use the consolidated property
  • Documentation: Updated README to be generic (not ViewEngine-specific) with examples for both ViewEngine and direct TemplateOptions usage

The callback is invoked only when templates are first parsed, not when retrieved from cache. The modified template is cached and reused on subsequent renders, improving performance. This consolidation provides a single, unified API for the TemplateParsed callback across both the ViewEngine and core library.

Original prompt

This section details on the original issue you should resolve

<issue_title>Emitting TemplateParsed events for templates called by render and include statements</issue_title>
<issue_description>This is a follow-up on #868

I think a bit of misunderstanding has happened: the main problem for me is the 2nd issue in the description, while the 1st is just making it worse😅

Previously, the visitor didn't get the content of templates added by include/render, and now, despite the TemplateParsed event from #869 making visitor usage much more convenient, the RenderStatement and IncludeStatement still don't emit the event before rendering the inner template.

I believe the event should be emitted somewhere between these two actions

if (!Parser.TryParse(content, out template, out var errors))
{
throw new ParseException(errors);
}
context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);

and these two
if (!Parser.TryParse(content, out template, out var errors))
{
throw new ParseException(errors);
}
context.Options.TemplateCache?.SetTemplate(relativePath, fileInfo.LastModified, template);

Example

File root.liquid:

{{ 1 | plus: 2 }}
{% render 'inner' %}

File inner.liquid:

{{ 2 | plus: 2 }}

after rewriting

 _options.TemplateParsed = (path, template) =>
{
    var visitor = new Fluid.Tests.Visitors.ReplaceTwosVisitor(Fluid.Values.NumberValue.Create(4));
    return visitor.VisitTemplate(template);
};

should return

5
8

But currently it's

5
4

Same goes to {% include 'inner' %}</issue_description>

<agent_instructions>Ensure new unit tests are implemented to verify the include and render tags correctly emit the TemplateParsed event. My asumption is that the RenderStatement and IncludeStatement classes can just invoke this event directly.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@sebastienros I completely missed the include/render part of your issue. Fix incoming thanks to copilot (hopefully).

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…d render statements

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
Copilot AI changed the title [WIP] Emit TemplateParsed events for render/include statements Emit TemplateParsed events for templates loaded by render and include statements Nov 20, 2025
Copilot AI requested a review from sebastienros November 20, 2025 15:36
@sebastienros

Copy link
Copy Markdown
Owner

@copilot Update FluidViewEngineOptions to use this new TemplateParsed callback instead of the one it has. Update existing usages to use the one in TemplateOptions now. Update the samples and documentation.

…wEngineOptions

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

@copilot Update FluidViewEngineOptions to use this new TemplateParsed callback instead of the one it has. Update existing usages to use the one in TemplateOptions now. Update the samples and documenta...

Done in commit 8e260eb.

Changes made:

  • Removed duplicate TemplateParsedDelegate and TemplateParsed from FluidViewEngineOptions
  • Updated FluidViewRenderer to use _fluidViewEngineOptions.TemplateOptions.TemplateParsed
  • Updated all tests in ViewEngineTests to use _options.TemplateOptions.TemplateParsed
  • Updated README documentation with examples for both ViewEngine usage (options.TemplateOptions.TemplateParsed) and direct TemplateOptions usage (options.TemplateParsed)

All 1327 tests pass.

Comment thread README.md Outdated
services.AddMvc().AddFluid(options =>
{
options.TemplateParsed = (path, template) =>
options.TemplateOptions.TemplateParsed = (path, template) =>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot update this section so it's not specific to the view engine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 480170a. Updated the section to cover all template parsing scenarios, not just ViewEngine. The documentation now shows examples for both ViewEngine usage and direct TemplateOptions usage with include/render statements.

Comment thread Fluid/Ast/IncludeStatement.cs Outdated
}
else if (context.Options.TemplateParsed != null)
{
// Template was retrieved from cache, but we still need to apply the callback

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Templates retrieved from cache should not emit the vent. Only when they are first parsed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 480170a. Removed the else if block that was invoking the callback for cached templates. Now the callback is only invoked when templates are first parsed (inside the main if block), not when retrieved from cache. Updated tests to verify the callback is called exactly once and the modified template is cached.

…; update docs

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
@sebastienros
sebastienros marked this pull request as ready for review November 20, 2025 16:12
Copilot AI requested a review from sebastienros November 20, 2025 16:14
Updated section on using visitors with templates for clarity.
@sebastienros
sebastienros enabled auto-merge (squash) November 20, 2025 16:18
@sebastienros
sebastienros merged commit d33047e into main Nov 20, 2025
3 checks passed
@sebastienros
sebastienros deleted the copilot/emit-template-parsed-events branch November 20, 2025 16:20
sebastienros added a commit that referenced this pull request Aug 20, 2026
… statements (#873)

* Initial plan

* Add TemplateParsed callback support to TemplateOptions for include and render statements

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>

* Consolidate TemplateParsed to use TemplateOptions instead of FluidViewEngineOptions

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>

* Fix TemplateParsed to only invoke on first parse, not cache retrieval; update docs

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>

* Revise visitors section in README

Updated section on using visitors with templates for clarity.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
Co-authored-by: Sébastien Ros <sebastienros@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ee9c8775-0354-41dd-8226-89a69c60241c
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 this pull request may close these issues.

Emitting TemplateParsed events for templates called by render and include statements

2 participants