fix liquid template argument parsing and shortcode undefined values

[epelc]

• fixes undefined arguments in shortcodes and potentially other uses
• fixes Shortcode arguments not passed correctly when called from liquid layout #2348 and Paginated layout removes arguments from liquid shortcode #2154

This fixes a major bug with liquid shortcodes not working in layout templates. They end up randomly getting undefined values for arguments after the first argument due to an order of execution issue with a changing scope context being sent to evalValue.

I changed it from running the promise using await immediately to calling the promise immediately instead so it gets the correct current scope value. I then use await Promise.all(pendingPromises) to wait for each promise to resolve.

There is a separate example here which shows the differences in order of execution with calling the promise immediately with await(broken) vs calling it without await and then later awaiting all of the promises(fixed).

Demo/why it fixes and didn't work before

See example javascript below

Notice how the regular version starts the two functions concurrently and their counter is then randomly iterated between the the functions. Consider each function an eleventy input file and the counter to be the scope in the liquid parser. You end up with the undefined arguments bug in this setup because the scope changes out of order compared to where the variables are being processed.

Compare with the fixed version under it in which the first file is completely executed then the next one. Nodejs is single threaded anyways so executing it in order synchronously while still using async/await is fine. We are just fixing the processing order here so that the counter/scope are processed in the corresponding order to their arguments.

> console.log('reg',await Promise.all([doConcurrentStuff(ctx),doConcurrentStuff(ctx)]))
Doing concurrent stuff
ctx { counter: 71 }
Doing concurrent stuff
ctx { counter: 72 }
ctx { counter: 73 }
ctx { counter: 74 }
ctx { counter: 75 }
ctx { counter: 76 }
ctx { counter: 77 }
ctx { counter: 78 }
ctx { counter: 79 }
ctx { counter: 80 }
reg [ [ 71, 73, 75, 77, 79 ], [ 72, 74, 76, 78, 80 ] ]
undefined
> console.log('fixed',await Promise.all([doConcurrentStufffixed(ctx),doConcurrentStufffixed(ctx)]))
Doing concurrent stuff with fixed passing of current value of ctx/scope
ctx { counter: 81 }
ctx { counter: 82 }
ctx { counter: 83 }
ctx { counter: 84 }
ctx { counter: 85 }
Doing concurrent stuff with fixed passing of current value of ctx/scope
ctx { counter: 86 }
ctx { counter: 87 }
ctx { counter: 88 }
ctx { counter: 89 }
ctx { counter: 90 }
fixed [ [ 81, 82, 83, 84, 85 ], [ 86, 87, 88, 89, 90 ] ]
undefined

Example.js

function calc(scope) {
  return new Promise((resolve) => {
    scope.counter += 1

    console.log('ctx', scope)
    resolve(scope.counter)
  })
}

async function doConcurrentStuff(ctx) {
  console.log('Doing concurrent stuff')
  let out = []
  for (let i = 0; i < 5; i++) {
    // ctx.counter += i
    out.push(await calc(ctx))
  }

  return out
}

async function doConcurrentStufffixed(ctx) {
  console.log(
    'Doing concurrent stuff with fixed passing of current value of ctx/scope'
  )

  let out = []
  for (let i = 0; i < 5; i++) {
    // ctx.counter =i+ctx.counter
    // ctx.counter += i

    out.push(calc(ctx))
  }

  return await Promise.all(out)
}

let ctx = {
  counter: 10,
}

console.log(
  'reg',
  await Promise.all([doConcurrentStuff(ctx), doConcurrentStuff(ctx)])
)
console.log(
  'fixed',
  await Promise.all([doConcurrentStufffixed(ctx), doConcurrentStufffixed(ctx)])
)

[pdehaan]

/summon @zachleat We might need this in a 1.0.2 release.

[epelc]

I would agree it should be in 1.0.2. It’s a pretty large bug and you can miss it since it’s random too.

Also you don’t hit it when you just test out eleventy with a single content file. It’s only after you have two or more so we didn’t get until further into switching our site over to eleventy.

We are fine for now since we install from my git repo to get this fix but it’d be nice for others.

[zachleat]

Merging, thank you so much! Excellent sleuthing.

This will go out with 2.0.0-canary.9 today and I’ll merge it over the 1.x branch too for the upcoming 1.0.2.

If you could, at some point, could you also PR a test for this?

[zachleat]

1.x baca2ad