Shopify / Shopify/theme-tools

theme-graph: parse failures silently produce an incomplete graph, causing false dead-code results

Open
#1,290 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
234
Forks
92
Avg merge
1d 3h
Merged PRs (30d)
6

Description

Summary

When a Liquid file fails to parse, buildThemeGraph silently emits that file as a node with zero dependencies instead of surfacing the parse failure. Every asset referenced only from the unparseable file is therefore never added to the graph, and any dead-code feature built on the graph reports those assets as unused.

The failure is completely silent: exit code 0, nothing on stderr, and the unparseable file still appears in the graph as a normal, reachable node — so there is no signal that the graph is incomplete.

This is the same observable failure mode as #1279 (graph incompleteness surfacing as false orphans), but a different trigger: a parse error rather than contextual templates.

Versions
  • @shopify/theme-graph 0.3.2
  • @shopify/theme-check-common 3.29.0
  • @shopify/liquid-html-parser 2.10.0
  • Node 24.11.1, Windows 11
Reproduction

Minimal theme (minitheme/):

templates/index.json

{ "sections": { "ok": { "type": "ok" }, "broken": { "type": "broken" } },
  "order": ["ok", "broken"] }

sections/ok.liquid — parses fine, references an asset

{{ 'used-by-ok.css' | asset_url | stylesheet_tag }}
<div>ok</div>
{% schema %}{"name":"ok"}{% endschema %}

sections/broken.liquid — references an asset, then fails to parse

{{ 'used-by-broken.css' | asset_url | stylesheet_tag }}
{% if true %}<div class="a {% endif %}">x</div>
{% schema %}{"name":"broken"}{% endschema %}

Plus layout/theme.liquid, config/settings_schema.json, locales/en.default.json, and three assets: used-by-ok.css, used-by-broken.css, genuinely-unused.css.

npx theme-graph minitheme > graph.json
Actual
exit 0
stderr: Preloading files took 7.7ms / Build graph took 3.7ms   (no error, no warning)

nodes: 8  edges: 6

IN GRAPH  used-by-ok.css
ABSENT    used-by-broken.css      <-- referenced, but reported as absent
ABSENT    genuinely-unused.css    <-- correct

sections/broken.liquid is in the graph and is reached (index.json -> broken.liquid [direct]), but contributes zero outbound edges.

Confirming the cause directly:

import { toLiquidHtmlAST } from '@shopify/liquid-html-parser';
toLiquidHtmlAST(fs.readFileSync('minitheme/sections/broken.liquid','utf8'));
// LiquidHTMLASTParsingError: Attempting to close LiquidTag 'if' before it was opened
//   without a matching 'if'
Expected

Either of these would be fine; the current behaviour is the problem:

  1. buildThemeGraph surfaces parse failures — a diagnostics/errors array on the returned graph, or a per-node parseError flag — so consumers can distinguish "this file has no dependencies" from "this file could not be read".
  2. The CLI exits non-zero, or at minimum writes a warning to stderr, listing files it could not parse.

The distinction matters because "no dependencies" and "unknown dependencies" are opposite conclusions for dead-code analysis, and right now they are indistinguishable.

Impact in practice

On a production Clean Canvas Enterprise 2.3.0 theme (315 nodes, 908 edges), 2 of 43 sections containing asset_url produced zero outbound edges, both because of LiquidHTMLASTParsingError on constructs that render correctly in production. That was enough to make several genuinely-referenced stylesheets look unused.

Worth noting for context: the parse failures themselves are on valid Liquid-inside-HTML that Shopify renders fine — the vendor theme ships this way. So a consumer cannot treat "unparseable" as "broken file the developer should fix"; unparseable-but-working files exist in the wild, which is exactly why the graph needs to report them rather than quietly dropping their edges.

Aside

Separately, and possibly a naming/docs issue rather than a bug: serialized nodes for CSS/JS assets carry kind: "unused" regardless of whether anything references them. In the run above, used-by-ok.css has kind: "unused" while having an inbound edge. On the production theme, 107 nodes carried kind: "unused" while 0 were genuinely unreferenced. It reads as a usage signal and is easy to mistake for one — happy to split this into its own issue if useful.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with buildThemeGraph and reproduce the issue using the provided minitheme and npx theme-graph minitheme > graph.json. Compare its behavior with the direct toLiquidHtmlAST parse failure, then trace how the CLI reports graph-building failures. Done means parse failures are distinguishable from files with no dependencies through the graph or CLI output.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
cli, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.