Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 22, 2025

Summary

Fixes the parser to allow combining with/as or for/as syntax with named arguments in the render tag, matching Shopify Liquid's behavior.

Problem

Previously, Fluid only accepted either the with/as syntax or named arguments, but not both together. This was inconsistent with Shopify Liquid, where combining them is valid and commonly used:

{%- render 'icon' with 'rating-star', class: 'rating__star' -%}

Attempting to parse this template resulted in:

Error: Invalid 'render' tag at (1:11)

Changes

1. Parser Enhancement (FluidParser.cs)

Modified the RenderTag parser to accept optional named arguments after with/as and for/as expressions:

// Before: with/as without named arguments
String.AndSkip(Terms.Text("with")).And(Primary).And(ZeroOrOne(Terms.Text("as").SkipAnd(Identifier)))

// After: with/as with optional named arguments
String.AndSkip(Terms.Text("with")).And(Primary).And(ZeroOrOne(Terms.Text("as").SkipAnd(Identifier)))
  .And(ZeroOrOne(Comma.SkipAnd(Separated(Comma, ...named arguments...))))

Reordered parser alternatives to check with/for before named-arguments-only to ensure correct precedence.

2. Statement Execution Fix (RenderStatement.cs)

Fixed a critical bug in the if/else branch order. When both For and AssignStatements were present, the code incorrectly took the assign-only branch instead of the for branch:

// Before: Wrong branch order
if (With != null) { ... }
else if (AssignStatements.Count > 0) { ... }  // ❌ Taken when For != null too!
else if (For != null) { ... }

// After: Correct branch order  
if (With != null) { ... }
else if (For != null) { ... }  // ✅ Checked before assign-only
else if (AssignStatements.Count > 0) { ... }

Added support for evaluating assign statements within both with and for branches while maintaining proper scope isolation.

3. Comprehensive Tests (IncludeStatementTests.cs)

Added 6 new tests covering all combinations:

  • with + named arguments
  • with + as + named arguments
  • with + multiple named arguments
  • for + named arguments
  • for + as + named arguments
  • Scope isolation verification

Examples Now Supported

{%- render 'icon' with 'rating-star', class: 'rating__star' -%}
{% render 'product' with my_product as p, price: '$99' %}
{% render 'button' with 'Click Me', size: 'large', color: 'blue' %}
{% render 'product' for products, tag: 'sale' %}
{% render 'item' for items as i, status: 'active' %}
{% render 'collection-product-list' with c, limit_count: 2, heading_text: 'All Products' %}

Testing

  • ✅ All 1,179 tests passing
  • ✅ No security vulnerabilities detected by CodeQL
  • ✅ Maintains backward compatibility with existing syntax
  • ✅ Matches Shopify Liquid behavior per official documentation and blog examples

Fixes #[issue_number]

Original prompt

This section details on the original issue you should resolve

<issue_title>The render tag should support both with/as and named arguments simultaneously</issue_title>
<issue_description>I’ve encountered what appears to be a parser bug in the render tag.
Currently, Fluid only accepts either the with/as syntax or named arguments, but not both together.
However, in standard Liquid (such as Shopify’s implementation), combining them is valid and widely used.

Example:

{%- render 'icon' with 'rating-star', class: 'rating__star' -%}

Observed behavior
Parsing this template results in the following error:

Error: Invalid 'render' tag at (1:11)
Source:
{%- render 'icon' with 'rating-star', class: 'rating__star' -%}

Expected behavior
This syntax should be allowed - the with expression should define the primary context value,
while additional named arguments (like class) should still be passed into the render scope.

For reference, Shopify's documentation on the render tag does not forbid this combination, and the official blog provides multiple examples that imply its support:

{% render 'collection-product-list' with c, limit_count: 2, heading_text: 'All Products' %}

Fluid version: 2.25.0</issue_description>

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

Fixes #830


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix render tag to support with/as and named arguments Support combining with/as and named arguments in render tag Oct 22, 2025

var options = new TemplateOptions() { FileProvider = fileProvider, MemberAccessStrategy = UnsafeMemberAccessStrategy.Instance };
var context = new TemplateContext(options);
_parser.TryParse("{% render 'snippet', class: 'test' %}{{ class }}", out var template);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_parser.TryParse("{% render 'snippet', class: 'test' %}{{ class }}", out var template);
_parser.TryParse("{% render 'snippet', class: 'test' %}", out var template);

I suppose we don't want {{ class }} here as it's not what we're checking for and leads to doubling output

Copy link
Owner

Choose a reason for hiding this comment

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

I think this test is actually checking that this is not available in the calling template, check the name of the test. And also the result (no dupliation)

@sebastienros sebastienros marked this pull request as ready for review October 22, 2025 18:45
@sebastienros sebastienros merged commit ef447b9 into main Oct 22, 2025
3 checks passed
@sebastienros sebastienros deleted the copilot/fix-render-tag-argument-support branch October 22, 2025 22:35
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.

The render tag should support both with/as and named arguments simultaneously

3 participants