Skip to content

Add TemplateParsed callback for AST visitor integration in ViewEngine - #869

Merged
sebastienros merged 5 commits into
mainfrom
copilot/support-ast-visitor-registration
Nov 20, 2025
Merged

Add TemplateParsed callback for AST visitor integration in ViewEngine#869
sebastienros merged 5 commits into
mainfrom
copilot/support-ast-visitor-registration

Conversation

Copilot AI commented Nov 19, 2025

Copy link
Copy Markdown
Contributor

Implementation Complete: Support for AST Visitors/Rewriters in ViewEngine

Successfully added a callback property in ViewEngine options that allows users to update a template once it is parsed, before it is cached.

Plan Checklist:

  • Explore repository structure and understand the codebase
  • Understand how AST visitors work in Fluid
  • Understand the ViewEngine architecture
  • Build and test the solution to ensure baseline
  • Add TemplateParsed callback to FluidViewEngineOptions
  • Update FluidViewRenderer.GetFluidTemplateAsync() to invoke the callback after parsing
  • Add tests for ViewEngine callback functionality
  • Update FluidMvcViewOptions (inherits automatically, no changes needed)
  • Run all tests to ensure no regressions (1320 tests passed)
  • Add documentation to README.md
  • Refactor CompositeFluidTemplate to implement IStatementList
  • Simplify tests and documentation (no special handling needed)
  • Fix CompositeFluidTemplate.RenderAsync() to use Statements instead of Templates
  • Add test to verify CompositeFluidTemplate renders altered statements correctly
  • Run security checks (No issues found)

Implementation Summary:

Added a TemplateParsed callback delegate to FluidViewEngineOptions that gets invoked after a template is parsed but before it's cached. This allows users to apply visitors/rewriters to modify templates as needed.

Refactored CompositeFluidTemplate to implement IStatementList by collecting all statements from inner templates. This simplifies the API - users no longer need special handling for CompositeFluidTemplate when applying visitors.

Fixed CompositeFluidTemplate.RenderAsync() to render using the Statements property instead of iterating through Templates. This ensures consistency with FluidTemplate and properly handles altered statements. The implementation now uses the same optimized pattern as FluidTemplate with synchronous fast path for better performance.

The callback is invoked in FluidViewRenderer.GetFluidTemplateAsync() after parsing but before caching, ensuring that:

  1. Modified templates are cached (addressing Issue 1: ViewEngine integration)
  2. The callback applies to all templates including partials and ViewStarts (addressing Issue 2: propagation to nested templates)

Changes Made:

  1. FluidViewEngineOptions.cs: Added TemplateParsedDelegate and TemplateParsed property
  2. FluidViewRenderer.cs: Updated GetFluidTemplateAsync() to invoke the callback after parsing, before caching
  3. CompositeFluidTemplate.cs:
    • Implemented IStatementList by collecting statements from inner templates
    • Changed RenderAsync() to use Statements property instead of Templates
    • Adopted optimized sync/async pattern from FluidTemplate for better performance
  4. ViewEngineTests.cs:
    • Simplified existing tests to directly use visitor.VisitTemplate() without special handling
    • Added test to verify CompositeFluidTemplate renders using Statements property
  5. README.md: Simplified documentation and usage examples

Simplified Usage Example:

services.AddMvc().AddFluid(options =>
{
    options.TemplateParsed = (path, template) =>
    {
        var visitor = new MyCustomVisitor();
        return visitor.VisitTemplate(template);
    };
});

The callback now works seamlessly with all template types, and CompositeFluidTemplate correctly renders altered statements.

Security Summary:

  • No security vulnerabilities detected by CodeQL
  • All changes are minimal and focused on the specific feature request
  • No breaking changes to existing functionality
Original prompt

This section details on the original issue you should resolve

<issue_title>Support for registering AST Visitors/Rewriters in ViewEngine</issue_title>
<issue_description>### Use Case

We want to get theme templates directly from Shopify, add a few additional attributes, and render them using a conversion of our model to Shopify's one. I'm trying to use AstRewriter to rewrite templates in the ViewEngine. There are two issues:

Issue 1: ViewEngine integration

The ViewEngine does not allow connecting any visitor/rewriter directly. Is there any way to do this except by inheriting FluidViewRenderer that I'm not aware of?

Currently, the only approach I've found is to override FluidViewRenderer.RenderViewAsync() or similar methods to manually apply visitors.

Issue 2: Visitors don't propagate to nested templates

Even when we use a visitor on a template by overriding FluidViewRenderer.RenderPartialAsync(), it's not passed further to templates injected by render/include statements. This means the only way to use it properly is to create and connect copies of both RenderStatement/IncludeStatement and eventually all the classes up the chain.

Proposal

From my point of view, it would be nice to register a visitor at FluidViewEngineOptions as it works with RegisterParserBlock at LiquidViewParser, which should be passed to the subtemplates (maybe optionally to keep compatibility) via TemplateContext.</issue_description>

<agent_instructions>Add a new callback property in the ViewEngine options such that users can update a template once it is parsed, before it is cached. Create tests for the ViewEngine and for the MvcViewEngine.</agent_instructions>

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

@sebastienros Devs can invoke visitors because they have access to the `IFluidTemplate`, but with ViewEngine the templates are not directly available, that is what you mean?

In that case it totally makes sense to expose these. But maybe registering them globaly doesn't make much sense, might be better to get a callback on the parse template such that you can do anything on it, even applying a visitor, or do it conditionally at least. TemplateParsed event for instance?

There could be a mitigation already though, using the TemplateCache, so when a new template is provided, alter it and save the alteration. But maybe the invoker is not getting the modified one ... might not work today either.</comment_new>


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

Copilot AI and others added 2 commits November 19, 2025 15:49
Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
Copilot AI changed the title [WIP] Add support for registering AST visitors in ViewEngine Add TemplateParsed callback for AST visitor integration in ViewEngine Nov 19, 2025
Copilot AI requested a review from sebastienros November 19, 2025 16:01
@sebastienros

Copy link
Copy Markdown
Owner

@copilot To simplify the samples that are checking for CompositeFluidTemplate, refactor the CompositeFluidTemplate class to implement public IStatementList. Base the implementation of FluidTemplate by storing the list of all statements from the inner templates.

…ify visitor usage

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

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

@copilot To simplify the samples that are checking for CompositeFluidTemplate, refactor the CompositeFluidTemplate class to implement public IStatementList. Base the implementation of `FluidTemp...

Done in commit 251952e. CompositeFluidTemplate now implements IStatementList by collecting all statements from inner templates. This simplifies the usage pattern - visitors can now be applied directly without checking for CompositeFluidTemplate.

The tests and documentation have been updated to use the simplified pattern:

options.TemplateParsed = (path, template) =>
{
    var visitor = new MyCustomVisitor();
    return visitor.VisitTemplate(template);
};

@sebastienros

Copy link
Copy Markdown
Owner

@copilot If a statement is altered by an AstRewriter then a CompositeFluidTemplate will fail because it's still renders the original templates and not the altered statements. Create a test to verify this assumption and then fix the implementation if necessary. Maybe use the same code as FluidTemplate which is also optimized for non-async code.

…luidTemplate

Co-authored-by: sebastienros <1165805+sebastienros@users.noreply.github.com>
@sebastienros
sebastienros marked this pull request as ready for review November 20, 2025 03:07
@sebastienros sebastienros reopened this Nov 20, 2025
Copilot AI requested a review from sebastienros November 20, 2025 03:08
@sebastienros
sebastienros enabled auto-merge (squash) November 20, 2025 03:09
@sebastienros
sebastienros merged commit 7dc1b7e into main Nov 20, 2025
9 checks passed
@sebastienros
sebastienros deleted the copilot/support-ast-visitor-registration branch November 20, 2025 03:12
sebastienros added a commit that referenced this pull request Aug 20, 2026
…#869)

* Initial plan

* Add TemplateParsed callback to FluidViewEngineOptions with tests

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

* Add documentation for TemplateParsed callback in README

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

* Refactor CompositeFluidTemplate to implement IStatementList and simplify visitor usage

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

* Fix CompositeFluidTemplate to render using Statements property like FluidTemplate

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

---------

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: 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.

Support for registering AST Visitors/Rewriters in ViewEngine

2 participants