Repository navigation
fix(typescript): follow Jinja2 truthiness and filter coercion in the nunjucks renderer - #584
Open
Arfath Ahmed Syed (ArfiArfath21) wants to merge 1 commit into
Conversation
…nunjucks renderer Nunjucks evaluates conditions with JavaScript truthiness, so an empty list or map took the true branch, unlike jinja2 and the subset spec (section 2.2). Its upper/lower/trim/join filters also threw TypeError on omitted inputs and on numbers, where jinja2 renders an empty string or the stringified value. Emit runtime.truthy around if/inline-if/not/and/or while a template is being compiled (restored afterwards, like the existing runtime hooks) and override the four filters to coerce first. Verified against jinja2 with a 10,000 template differential fuzz run.
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.
Fixes #583
The TypeScript
jinja2key resolves toNunjucksRenderer, which diverged from the Jinja2 reference on two REQUIRED subset behaviours:{% if docs %}took the true branch for[]and{}, because nunjucks compiles conditions to JavaScript truthiness (spec/jinja-subset.md §2.2).trim/joinon an omitted input andupper/lower/trimon a number threwTypeError, where Jinja2 renders""or the stringified value (§2.1, §2.5).What changed, all in
runtime/typescript/packages/core/src/renderers/nunjucks.ts:renderSafelycompiles and runs a template,if, inlineif,not,andandorare emitted around aruntime.truthycheck that follows Jinja2 (empty list, map and string are falsy).and/orstill return an operand, as in Jinja2. The compiler methods andruntime.truthyare put back in afinally, the same way the existingmemberLookup/callWraphooks already are, so other nunjucks users in the process are untouched.upper,lower,trimandjoinare overridden throughenv.addFilterto coerce first (undefined / null become"", numbers and booleans stringify,joinaccepts strings and maps the way Jinja2 does).The sandbox hooks, the renderer key and the nonce handling are unchanged.
Tests (
tests/renderers.test.ts): 23 new cases for truthiness (every falsy and truthy shape,elif,not/and/or, inlineif, the guard-a-loop scenario), filter coercion, and restoring nunjucks afterwards, including after a template error. Their expected values are what Python'sjinja2renders. 9 of them fail onmainand all pass here.Verification:
vitestinpackages/core: 1257 passed.tsc --noEmitandnpm run format:checkare clean.jinja2: 6,000 random subset templates, then 10,000 more with empty-collection conditions, all identical after the change (before it, 167 of the 6,000 in the narrowest run differed). The Python and TSjinja_subsetengines already matchedjinja2on all of them.Happy to follow up with shared
rendervectors for empty-collection truthiness and undefined-through-filter, so the other runtimes are held to the same behaviour, if you want them.