Repository navigation
Parse Object.prototype names as ordinary names in liquid-html-parser - #1331
Open
PhilippeCollin wants to merge 1 commit into
Open
PhilippeCollin wants to merge 1 commit into
PhilippeCollin wants to merge 1 commit into
Conversation
`isLiteralKeyword` used `s in LiquidLiteralValues`, and `tagForName`
read `this.builtins[name]` from a plain object. Both also see inherited
properties, so `{{ constructor }}` became a LiquidLiteral whose value
was the Object function, and `{% constructor %}` / `{% __proto__ %}`
resolved to a non-tag "definition" and threw.
Recognize literal keywords with a switch (also faster than `in` on this
hot path) and store builtin tags in a Map. A test checks every
LiquidLiteralValues key is still recognized.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The parser looks up tag names and keywords in plain JavaScript objects. Plain objects come with built-in properties such as
constructorandtoString. So when a template uses one of those names, the parser finds the JavaScript built-in instead of treating it as an ordinary name:Inside
{% liquid %}, the same names leave the whole block unparsed. This happens in two places:isLiteralKeyword(checking fortrue,niland the other literals) andEnvironment.tagForName(looking up built-in tags).Templates rarely use these names, but
theme-checkand the Prettier plugin should treat them like any other unknown name, not crash.What
switchon the six keywords. A new test checks that every key ofLiquidLiteralValuesis still recognized, so the two can't drift apart silently.Map(Environmentisn't exported from the package).{{ constructor }}now parses as a variable, and{% constructor %}parses as an unknown tag, both in documents and in{% liquid %}lines.The
switchis also faster thaninon this hot path. Parsing the 429 fixture files (Dawn, Horizon, base theme; M3 Pro, Node 24.15; median of 5 alternating rounds × 30 passes) saves about 2 ms per pass:toLiquidHtmlAST, all 429 filesmainOther ASTs are unchanged: a differential run against
mainover about 25,700 inputs (every.liquidfile in the repo, plus random and mutated templates) found 0 differences.liquid-html-parser,prettier-plugin-liquidandtheme-check-commontests pass.Related: #1329, #1330. Found while profiling after the tokenizer change.