Skip to content

Support arbitrary params in UniqueStaticBlockId duplicate detection#1162

Merged
graygilmore merged 1 commit intomainfrom
gg-unique-static-block-id-variant-awareness
Mar 27, 2026
Merged

Support arbitrary params in UniqueStaticBlockId duplicate detection#1162
graygilmore merged 1 commit intomainfrom
gg-unique-static-block-id-variant-awareness

Conversation

@graygilmore
Copy link
Copy Markdown
Contributor

@graygilmore graygilmore commented Mar 25, 2026

Summary

The UniqueStaticBlockId check was producing false positives on the Horizon theme because it only compared the id argument when detecting duplicate static blocks. Horizon's header intentionally renders the same block (header-menu) three times with different arbitrary params (variant: "mobile", variant: "navigation_bar") to produce desktop, mobile, and navigation bar versions from a single set of merchant-configurable settings.

The fix builds a composite key from the block id + all arbitrary params (excluding framework args like id, type, and reserved names). Two blocks are only flagged as duplicates if they share the same id AND identical arbitrary params. When any arbitrary param value is a variable lookup (not a string literal), the block is excluded from duplicate detection since its identity cannot be statically determined.

🎩 Tophatting

  1. Check out this branch and run yarn && yarn build

  2. Open VS Code in the theme-tools directory and press F5 to launch the Extension Development Host

  3. In the Extension Development Host, open a theme project (or create a test .liquid file in a theme directory)

  4. Paste the following into a section file (e.g. sections/test.liquid):

    {% content_for 'block', type: 'text', id: 'header-menu' %}
    {% content_for 'block', type: 'text', id: 'header-menu', variant: 'mobile' %}
    {% content_for 'block', type: 'text', id: 'header-menu', variant: 'navigation_bar' %}

    Expected: No errors — each call has different arbitrary params

  5. Now paste this instead:

    {% content_for 'block', type: 'text', id: 'header-menu', variant: 'mobile' %}
    {% content_for 'block', type: 'text', id: 'header-menu', variant: 'mobile' %}

    Expected: Error on the second line — The id 'header-menu' is already being used by another static block

  6. Verify the original duplicate detection still works (no arbitrary params):

    {% content_for 'block', type: 'text', id: 'static-block' %}
    {% content_for 'block', type: 'text', id: 'static-block' %}

    Expected: Error on the second line

@graygilmore graygilmore force-pushed the gg-unique-static-block-id-variant-awareness branch 2 times, most recently from a1763d4 to e812eaf Compare March 25, 2026 22:53
@graygilmore graygilmore changed the title Make UniqueStaticBlockId check variant-aware Support arbitrary params in UniqueStaticBlockId duplicate detection Mar 25, 2026
@graygilmore graygilmore force-pushed the gg-unique-static-block-id-variant-awareness branch 2 times, most recently from cb5b921 to b6e5a60 Compare March 25, 2026 23:32
@graygilmore graygilmore marked this pull request as ready for review March 26, 2026 17:15
@graygilmore graygilmore requested a review from a team as a code owner March 26, 2026 17:15
Copy link
Copy Markdown
Contributor

@aswamy aswamy left a comment

Choose a reason for hiding this comment

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

When any arbitrary param value is a variable lookup (not a string literal), the block is excluded from duplicate detection since its identity cannot be statically determined.

Good edge case catch. That was going to be my first question!

Tophat went well. Code is sound 👍

Image


const offenses = await runLiquidCheck(UniqueStaticBlockId, sourceCode);

expect(offenses).to.have.length(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you also add a test when two variable lookups (with the same variable) is used?

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.

@aswamy
Copy link
Copy Markdown
Contributor

aswamy commented Mar 27, 2026

@graygilmore just remember to update the examples on shopify-dev

The Horizon theme renders the same static block 3x with different arbitrary params (variant). The check was flagging false duplicates.

The fix builds a composite key from the block id + all arbitrary params (excluding framework args id, type, and reserved names). Two blocks are only flagged as duplicates if they share the same id AND identical arbitrary params.

Fixes #1132

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@graygilmore graygilmore force-pushed the gg-unique-static-block-id-variant-awareness branch from b6e5a60 to e10da7c Compare March 27, 2026 15:43
@graygilmore graygilmore merged commit c991874 into main Mar 27, 2026
8 checks passed
@graygilmore graygilmore deleted the gg-unique-static-block-id-variant-awareness branch March 27, 2026 15:49
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.

🪲 UniqueStaticBlockId false positive on official Horizon theme — duplicate id is intentional (variant parameter)

2 participants