[LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams #831

jamesmengo · 2025-02-27T20:37:08Z

What are you adding in this PR?

Closes https://github.com/Shopify/developer-tools-team/issues/605

Adds alias support to the UnrecognizedRenderSnippetParams theme check

If the alias used is unrecognized, we will report this.
We also had to handle some edge cases for removal since this follows a different format.

What's next? Any followup issues?

Need to update type checking next

Before you deploy

I included a minor bump changeset
My feature is backward compatible

jamesmengo · 2025-02-27T20:37:28Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

jamesmengo · 2025-02-27T23:08:03Z

packages/theme-check-common/src/checks/unrecognized-render-snippet-params/index.ts

@@ -69,9 +69,42 @@ export const UnrecognizedRenderSnippetParams: LiquidCheckDefinition = {
      }
    }

+    function reportUnknownAliases(


This is handled quite differently that Named Parameters so I split this into its own function

jamesmengo · 2025-02-27T23:10:38Z

packages/theme-check-common/src/checks/unrecognized-render-snippet-params/index.ts

+      snippetName: string,
+    ) {
+      if (node.alias && !liquidDocParameters.has(node.alias) && node.variable) {
+        const asAliasMatch = node.source.match(new RegExp(`as\\s+${node.alias}`));


This is a bit of a hack to strip as alias since we don't have that information in the AST right now. Luckily, it's a consistent pattern so this should work sufficiently.

I'm creating a separate ticket to address the parsing work, etc that would need to be done to do this with the node positions.

jamesmengo · 2025-02-27T23:31:29Z

packages/theme-check-common/src/checks/unrecognized-render-snippet-params/index.ts

+        context.report({
+          message: `Unknown parameter '${node.alias}' in render tag for snippet '${snippetName}'`,
+          startIndex: node.variable.position.start,
+          endIndex: node.variable.position.end,


Right now this will only highlight **with 'product'** as alias . This is due to the current capabilities of the parsing for render markup.

Tracked an issue here ( same as the one mentioned above)

Suggested change

endIndex: node.variable.position.end,

endIndex: asAliasMatch ? node.variable.position.end + asAliasMatch[0].length : node.variable.position.end,

What do you think about adding asAliasMatch[0].length to the endIndex?

We could also add an offset to startIndex and endIndex to avoid spaces:

const startingOffset = node.source.slice(node.variable.position.start).indexOf(node.variable.kind); ... context.report({ ... startIndex: startingOffset + node.variable.position.start, endIndex: startingOffset + (asAliasMatch ? node.variable.position.end + asAliasMatch[0].length : node.variable.position.end), })

great idea!

- Update check to handle 'as' alias scenarios - Adjust fixer to correctly insert missing parameters

aswamy

~~Optional suggestions for improving as alias~~

1 optional change
1 mandatory change with node.source replace

aswamy · 2025-02-28T20:44:39Z

packages/theme-check-common/src/checks/unrecognized-render-snippet-params/index.ts

+                  if (node.variable) {
+                    return fixer.remove(
+                      node.variable.position.start,
+                      node.source.indexOf(asAliasMatch[0]) + asAliasMatch[0].length,


Suggested change

node.source.indexOf(asAliasMatch[0]) + asAliasMatch[0].length,

node.source.slice(node.position.start).indexOf(asAliasMatch[0]) + asAliasMatch[0].length,

This is a bit dangerous. I had two render tags with as foo in it. node.source returns the entire file, so doing a indexOf could find the wrong one.

aswamy · 2025-02-28T20:50:54Z

packages/theme-check-common/src/checks/unrecognized-render-snippet-params/index.ts

+        context.report({
+          message: `Unknown parameter '${node.alias}' in render tag for snippet '${snippetName}'`,
+          startIndex: node.variable.position.start,
+          endIndex: node.variable.position.end,


Suggested change

endIndex: node.variable.position.end,

endIndex: asAliasMatch ? node.variable.position.end + asAliasMatch[0].length : node.variable.position.end,

What do you think about adding asAliasMatch[0].length to the endIndex?

We could also add an offset to startIndex and endIndex to avoid spaces:

const startingOffset = node.source.slice(node.variable.position.start).indexOf(node.variable.kind); ... context.report({ ... startIndex: startingOffset + node.variable.position.start, endIndex: startingOffset + (asAliasMatch ? node.variable.position.end + asAliasMatch[0].length : node.variable.position.end), })

- Handle multiple render tags - Expand error reporting to highlight entire alias if found

jamesmengo · 2025-02-28T23:03:14Z

Merge activity

Feb 28, 6:03 PM EST: A user started a stack merge that includes this pull request via Graphite.
Feb 28, 6:03 PM EST: A user merged this pull request with Graphite.

jamesmengo mentioned this pull request Feb 27, 2025

[Refactor] Split ValidRenderSnippetParams into separate checks for missing and unrecognizeid parameters #829

Merged

6 tasks

jamesmengo mentioned this pull request Feb 27, 2025

[LiquidDoc][Theme Check] Add alias support for MissingRenderSnippetParams #830

Merged

2 tasks

jamesmengo changed the base branch from jm/aliases_missing_render_snippets to graphite-base/831 February 27, 2025 20:39

jamesmengo mentioned this pull request Feb 27, 2025

Delete validRenderSnippetParams files #832

Merged

jamesmengo changed the title ~~--wip-- [skip ci]~~ [LiquidDoc][Theme Check] Report unknown parameters in render tag aliases Feb 27, 2025

jamesmengo force-pushed the jm/alias_unrecognized_params branch from 376a29e to 0b1bbf4 Compare February 27, 2025 21:03

jamesmengo changed the title ~~[LiquidDoc][Theme Check] Report unknown parameters in render tag aliases~~ [LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams Feb 27, 2025

jamesmengo added the #gsd:44310 label Feb 27, 2025 — with Graphite App

jamesmengo requested a review from aswamy February 27, 2025 21:07

jamesmengo marked this pull request as ready for review February 27, 2025 21:07

jamesmengo requested a review from a team as a code owner February 27, 2025 21:07

jamesmengo force-pushed the jm/alias_unrecognized_params branch from 0b1bbf4 to 51a2db1 Compare February 27, 2025 21:09

jamesmengo changed the base branch from graphite-base/831 to jm/aliases_missing_render_snippets February 27, 2025 21:09

jamesmengo force-pushed the jm/alias_unrecognized_params branch from 51a2db1 to 619f11e Compare February 27, 2025 21:10

jamesmengo force-pushed the jm/aliases_missing_render_snippets branch 2 times, most recently from 79481f6 to 6ce5eff Compare February 27, 2025 22:14

jamesmengo force-pushed the jm/alias_unrecognized_params branch from 619f11e to e7f9907 Compare February 27, 2025 22:14

jamesmengo force-pushed the jm/aliases_missing_render_snippets branch from 6ce5eff to 013ca06 Compare February 27, 2025 22:59

jamesmengo force-pushed the jm/alias_unrecognized_params branch from e7f9907 to ad1f83a Compare February 27, 2025 23:03

jamesmengo commented Feb 27, 2025

View reviewed changes

jamesmengo mentioned this pull request Feb 28, 2025

Add type checking support for render snippets using alias syntax #833

Merged

2 tasks

jamesmengo changed the base branch from jm/aliases_missing_render_snippets to graphite-base/831 February 28, 2025 20:06

jamesmengo force-pushed the jm/alias_unrecognized_params branch from ad1f83a to 99aba80 Compare February 28, 2025 20:06

jamesmengo changed the base branch from graphite-base/831 to main February 28, 2025 20:06

Add alias with/for syntax support to MissingRenderSnippetParamsCheck

2135423

- Update check to handle 'as' alias scenarios - Adjust fixer to correctly insert missing parameters

jamesmengo force-pushed the jm/alias_unrecognized_params branch from 99aba80 to 2135423 Compare February 28, 2025 20:06

aswamy approved these changes Feb 28, 2025

View reviewed changes

Improve index calculation for 'as' alias removal suggestions

3f1505c

- Handle multiple render tags - Expand error reporting to highlight entire alias if found

jamesmengo merged commit d93bdc7 into main Feb 28, 2025
7 checks passed

jamesmengo deleted the jm/alias_unrecognized_params branch February 28, 2025 23:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams #831

[LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams #831

jamesmengo commented Feb 27, 2025 •

edited

Loading

jamesmengo commented Feb 27, 2025 •

edited

Loading

jamesmengo Feb 27, 2025

jamesmengo Feb 27, 2025 •

edited

Loading

jamesmengo Feb 27, 2025

aswamy Feb 28, 2025

jamesmengo Feb 28, 2025

aswamy left a comment •

edited

Loading

aswamy Feb 28, 2025

aswamy Feb 28, 2025

jamesmengo commented Feb 28, 2025 •

edited

Loading

	endIndex: node.variable.position.end,
	endIndex: asAliasMatch ? node.variable.position.end + asAliasMatch[0].length : node.variable.position.end,

	node.source.indexOf(asAliasMatch[0]) + asAliasMatch[0].length,
	node.source.slice(node.position.start).indexOf(asAliasMatch[0]) + asAliasMatch[0].length,

[LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams #831

[LiquidDoc][Theme Check] Add alias support for UnrecognizedRenderSnippetParams #831

Conversation

jamesmengo commented Feb 27, 2025 • edited Loading

What are you adding in this PR?

What's next? Any followup issues?

Before you deploy

jamesmengo commented Feb 27, 2025 • edited Loading

jamesmengo Feb 27, 2025

Choose a reason for hiding this comment

jamesmengo Feb 27, 2025 • edited Loading

Choose a reason for hiding this comment

jamesmengo Feb 27, 2025

Choose a reason for hiding this comment

aswamy Feb 28, 2025

Choose a reason for hiding this comment

jamesmengo Feb 28, 2025

Choose a reason for hiding this comment

aswamy left a comment • edited Loading

Choose a reason for hiding this comment

aswamy Feb 28, 2025

Choose a reason for hiding this comment

aswamy Feb 28, 2025

Choose a reason for hiding this comment

jamesmengo commented Feb 28, 2025 • edited Loading

Merge activity

jamesmengo commented Feb 27, 2025 •

edited

Loading

jamesmengo commented Feb 27, 2025 •

edited

Loading

jamesmengo Feb 27, 2025 •

edited

Loading

aswamy left a comment •

edited

Loading

jamesmengo commented Feb 28, 2025 •

edited

Loading