Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: in conditionals, don't render anything after an else branch #671

Merged
merged 6 commits into from
Feb 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/tags/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ export default class extends Tag {
this.elseTemplates = []

let p: Template[] = []
let elseCount = 0
const stream: ParseStream = this.liquid.parser.parseStream(remainTokens)
.on('tag:when', (token: TagToken) => {
if (elseCount > 0) {
return
}

p = []

const values: ValueToken[] = []
Expand All @@ -29,9 +34,16 @@ export default class extends Tag {
templates: p
})
})
.on('tag:else', () => (p = this.elseTemplates))
.on('tag:else', () => {
elseCount++
p = this.elseTemplates
})
.on('tag:endcase', () => stream.stop())
.on('template', (tpl: Template) => p.push(tpl))
.on('template', (tpl: Template) => {
if (p !== this.elseTemplates || (p === this.elseTemplates && elseCount === 1)) {
harttle marked this conversation as resolved.
Show resolved Hide resolved
p.push(tpl)
}
})
.on('end', () => {
throw new Error(`tag ${tagToken.getText()} not closed`)
})
Expand Down
28 changes: 21 additions & 7 deletions src/tags/if.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,33 @@ export default class extends Tag {

constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
super(tagToken, remainTokens, liquid)
let p
let p: Template[] = []
joel-hamilton marked this conversation as resolved.
Show resolved Hide resolved
let elseCount = 0
liquid.parser.parseStream(remainTokens)
.on('start', () => this.branches.push({
value: new Value(tagToken.args, this.liquid),
templates: (p = [])
}))
.on('tag:elsif', (token: TagToken) => this.branches.push({
value: new Value(token.args, this.liquid),
templates: (p = [])
}))
.on('tag:else', () => (p = this.elseTemplates))
.on('tag:elsif', (token: TagToken) => {
if (elseCount > 0) {
p = []
return
}
this.branches.push({
value: new Value(token.args, this.liquid),
templates: (p = [])
})
})
.on('tag:else', () => {
elseCount++
p = this.elseTemplates
})
.on('tag:endif', function () { this.stop() })
.on('template', (tpl: Template) => p.push(tpl))
.on('template', (tpl: Template) => {
if (p !== this.elseTemplates || (p === this.elseTemplates && elseCount === 1)) {
harttle marked this conversation as resolved.
Show resolved Hide resolved
p.push(tpl)
}
})
.on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
.start()
}
Expand Down
30 changes: 22 additions & 8 deletions src/tags/unless.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,35 @@ export default class extends Tag {
elseTemplates: Template[] = []
constructor (tagToken: TagToken, remainTokens: TopLevelToken[], liquid: Liquid) {
super(tagToken, remainTokens, liquid)
let p
let p: Template[] = []
let elseCount = 0
this.liquid.parser.parseStream(remainTokens)
.on('start', () => this.branches.push({
value: new Value(tagToken.args, this.liquid),
test: isFalsy,
templates: (p = [])
}))
.on('tag:elsif', (token: TagToken) => this.branches.push({
value: new Value(token.args, this.liquid),
test: isTruthy,
templates: (p = [])
}))
.on('tag:else', () => (p = this.elseTemplates))
.on('tag:elsif', (token: TagToken) => {
if (elseCount > 0) {
p = []
return
}
this.branches.push({
value: new Value(token.args, this.liquid),
test: isTruthy,
templates: (p = [])
})
})
.on('tag:else', () => {
elseCount++
p = this.elseTemplates
})
.on('tag:endunless', function () { this.stop() })
.on('template', (tpl: Template) => p.push(tpl))
.on('template', (tpl: Template) => {
if (p !== this.elseTemplates || (p === this.elseTemplates && elseCount === 1)) {
harttle marked this conversation as resolved.
Show resolved Hide resolved
p.push(tpl)
}
})
.on('end', () => { throw new Error(`tag ${tagToken.getText()} not closed`) })
.start()
}
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/issues.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,20 @@ describe('Issues', function () {
const result = engine.parseAndRenderSync('{{ÜLKE}}', { ÜLKE: 'Türkiye' })
expect(result).toEqual('Türkiye')
})
it('#670 Should not render anything after an else branch', () => {
const engine = new Liquid()
const result = engine.parseAndRenderSync('{% assign value = "this" %}' +
'{% if false %}don\'t show' +
'{% else %}show {{ value }}' +
'{% else %}don\'t show{% endif %}', {})
expect(result).toEqual('show this')
})
it('#672 Should not render an elseif after an else branch', () => {
const engine = new Liquid()
const result = engine.parseAndRenderSync('{% if false %}don\'t show' +
'{% else %}show' +
'{% elsif true %}don\'t show' +
'{% endif %}', {})
expect(result).toEqual('show')
})
})
28 changes: 28 additions & 0 deletions test/integration/tags/case.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,32 @@ describe('tags/case', function () {
const html = await liquid.parseAndRender(src)
return expect(html).toBe('and or or')
})
it('should not render anything after an else branch', async function () {
const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
'{% case true %}' +
'{% when false %}don\'t show' +
'{% else %}show {{ value }}' +
'{% else %}don\'t show' +
'{% endcase %}', {})
expect(html).toEqual('show this')
})
it('should not render anything after an else branch even when first else branch is empty', async function () {
const html = await liquid.parseAndRenderSync('{% case true %}' +
'{% when false %}don\'t show' +
'{% else %}' +
'{% else %}don\'t show' +
'{% endcase %}', {})
expect(html).toEqual('')
})
it('should not render anything after an else branch even when there are \'when\' conditions', () => {
const engine = new Liquid()
const result = engine.parseAndRenderSync('{% assign value = "this" %}' +
'{% case true -%}' +
'{% when false -%}don\'t show' +
'{% else %}show {{ value }}' +
'{% else %}don\'t show' +
'{%- when true -%}don\'t show' +
'{%- endcase %}', {})
expect(result).toEqual('show this')
})
})
8 changes: 8 additions & 0 deletions test/integration/tags/if.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,12 @@ describe('tags/if', function () {
const html = await liquid.parseAndRender(src, scope)
return expect(html).toBe('success')
})
it('should not render anything after an else branch even when first else branch is empty', () => {
const engine = new Liquid()
const result = engine.parseAndRenderSync('{% if false %}don\'t show' +
'{% else %}' +
'{% else %}don\'t show' +
'%{% endif %}', {})
expect(result).toEqual('')
})
})
23 changes: 23 additions & 0 deletions test/integration/tags/unless.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,29 @@ describe('tags/unless', function () {
Inside wonderland
After wonderland`)
})
it('should not render anything after an else branch', async function () {
const html = await liquid.parseAndRenderSync('{% assign value = "this" %}' +
'{% unless true %}don\'t show' +
'{% else %}show {{ value }}' +
'{% else %}don\'t show' +
'{% endunless %}', {})
expect(html).toEqual('show this')
})
it('should not render anything after an else branch even when first else branch is empty', async function () {
const html = await liquid.parseAndRenderSync('{% unless true %}don\'t show' +
'{% else %}' +
'{% else %}don\'t show' +
'{% endunless %}', {})
expect(html).toEqual('')
})
it('should not render an elseif after an else branch', () => {
const engine = new Liquid()
const result = engine.parseAndRenderSync('{% unless true %}don\'t show' +
'{% else %}show' +
'{% elsif true %}don\'t show' +
'{% endunless %}', {})
expect(result).toEqual('show')
})

describe('sync support', function () {
it('should render else when predicate yields true', function () {
Expand Down