Inner function breaks outer rest parameters

[efdknittlfrank]

Great work on k6 0.34.0 🍾! (I was already impatiently waiting for all the new goodies :))

I upgraded my environment and noticed that several of my tests are now failing. Analyzing the failed tests, I noticed a very peculiar and strange interaction of rest parameters with inner function declarations. The repro can be reduced to just a few lines, which I have attached at the bottom of this issue.

I assume this is more likely to be a bug in goja than in k6 and most probably was introduced by #2109. Since k6 is the only way I can test and run JS in goja at the moment, I'm posting the bug here (also, because this has worked before in k6 up until and including version 0.33.0. It stopped working with 0.34.0).

On a related note, I expected startup of tests to be faster with the new version (due to less work being done by babel), but in fact I was observing a small increase in startup times. At least this is what k6 is telling me when being run with the -v flag. On the other hand, the time is printed next to "Babel: transformed", so perhaps the transformation is slower now?

Environment

• k6 version: 0.34.0
• OS and version: Windows 10
• Docker version and image, if applicable: loadimpact/k6:0.34.0

Expected Behavior

Rest parameters work, regardless of function body

Actual Behavior

Rest parameters stop working (empty array) as soon as the function contains an inner function (named, anonymous, or arrow) which references the rest param array.

Steps to Reproduce the Problem

function working(...rest) {
  console.log(rest.length);
}

function broken(...rest) {
  console.log(rest.length);
  () => rest;
}

function workaround(...rest) {
  console.log(rest.length);
  const alias = rest;
  () => alias;
}

export default function() {
  working(4,2); // 2
  broken(4,2); // 0
  workaround(4,2); // 2
}

[na--]

Thanks for creating this issue and sorry about the regression! You are quite likely right that it's because of the latest goja changes. 😞

We'll investigate and hopefully create a fix soon. It's also quite interesting that the Babel transformation times became longer when we disabled some of the plugins... 😕 Certainly deserves some investigation, it might be the case that the new goja version itself is slower in executing Babel, we have to check 🤷‍♂️

[mstoykov]

@efdknittlfrank can you please try #2135 and see if it fixes the issue in your original script?

[efdknittlfrank]

@mstoykov thanks for the fast PR :) no need to be sorry, regressions happen. Thanks for your work on k6; it is a really awesome tool!

Maybe I'm blind, but where can I download a build? #2135 seems to have failing tests, maybe that's why it is skipping creating packages?

I'd assume if the examples from this bug report are fixed with the fixed goja version, my original script would also work. For what's it worth, here is the function from my script:

export function f(...cbs) {
  return () => Object.assign({}, ...callbacks.map(cb => cb()));
}

f(
  () => doSomething(),
  () => 1+2,
  () => {}
);

[mstoykov]

@efdknittlfrank you need to build it yourself, the ci doesn't build releases for not ... releases ;) But here is a windows build
k6-pr2135.exe.zip

I tried your snippet (with some changes):

export function f(...cbs) {
          return () => Object.assign({}, ...cbs.map(cb => cb()));
}

console.log(JSON.stringify(f(
          () => "some",
          () => {return {"a": "123"}}
)()));

and am now getting {"0":"s","1":"o","2":"m","3":"e","a":"123"} with both v0.33.0 and the PR one. I still would prefer for you to run your whole script to see if it works in that case as well ;)

[efdknittlfrank]

@mstoykov that was … fast! Thanks. First impression: it works! 🎉