Faulty compression leads to arrays with holes / Removes assignments passed by value

[icyJoseph]

Describe the bug

Alright, I've got my case:

function foo() {
  let walker = 0;

  let arr = [];

  function bar(defaultValue) {
    const myIndex = walker;
    walker += 1;

    console.log({ arr });

    if (arr.length < myIndex + 1) {
      arr[myIndex] = defaultValue;
    }
  }

  return bar;
}

const bar = foo();

bar(null);
bar(null);
bar(null);
bar(null);
bar(null);

When compiled with swc using this .swcrc file:

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript"
    },
    "target": "es5",
    "minify": {
      "compress": true,
      "mangle": true
    }
  }
}

Outputs code that causes the walker to +1 before the if-statement that assigns the first default value. So for the first item we are setting the index 1, which creates a whole on the array.

var l = (function () {
  var l = 0,
    n = [];
  return function (u) {
    (l += 1),
      console.log({
        arr: n,
      }),
      n.length < l + 1 && (n[l] = u);
  };
})();
l(null), l(null), l(null), l(null), l(null);

The final log outputs: { arr: [ <1 empty item>, null, null, null, null ] }, but since the very beginning there's a hole at the beginning of the array.

package.json used:

{
  "name": "swc-test",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "devDependencies": {
    "@swc/cli": "^0.1.57",
    "@swc/core": "^1.3.4"
  }
}

If we set "compress": false, on the .swcrc file, then the bug goes away, aka the last log would be : { arr: [ null, null, null, null ] } - no log ever shows the hole in the array.

What does this mean for Next.js? Well that perhaps a new version with a fixed version of SWC is needed to be able to use the minifier. I'll try to report the bug to SWC as best as I can so that they can fix it.

This was first observed with libraries that use this custom React hook: https://github.com/react-component/overflow/blob/master/src/hooks/useBatchFrameState.tsx - You see the core of the issue is that:

    const myIndex = walkingIndex;
    walkingIndex += 1;

    // Fill value if not exist yet
    if (statesRef.current.length < myIndex + 1) {
      statesRef.current[myIndex] = defaultValue;
    }

The compression removes myIndex and replaces it with walkingIndex, which means that by the time the if statement runs, the indexing value is at 1, and we set defaultValue as second place, leaving a whole behind in the array. Reference: vercel/next.js#41125

Input code

function foo() {
  let walker = 0;

  let arr = [];

  function bar(defaultValue) {
    const myIndex = walker;
    walker += 1;

    console.log({ arr });

    if (arr.length < myIndex + 1) {
      arr[myIndex] = defaultValue;
    }
  }

  return bar;
}

const bar = foo();

bar(null);
bar(null);
bar(null);
bar(null);
bar(null);

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript"
    },
    "target": "es5",
    "minify": {
      "compress": true,
      "mangle": true
    }
  }
}

Expected behavior

Same behaviour between jsc.minify.compress set to false and set to true. In this case, that when the variable can be passed as value, and not reference, the compressor should not optimise away the intermediate holder.

var a = n;
n += 1;

In the above, a ought to survive compression because it gets passed n by value.

Actual behavior

The compressor incorrectly removes the variable that received it's assignment by value, as if it has been done by reference, which impacts actual runtime behaviour.

Version

1.3.4

Additional context

No response

[icyJoseph]

A bit more info:

• Changing bar to an arrow function fixes the issue

let arr = [];

function foo() {
  let walker = 0;
  // this works
  const bar = (defaultValue) => {
    const myIndex = walker;
    walker += 1;

    if (arr.length < myIndex + 1) {
      arr[myIndex] = defaultValue;
    }
  }

  return bar
}

const bar = foo();
bar(null);
console.log(arr); // [null]

• Directly returning the function also works

let arr = [];

function foo() {
  let walker = 0;
  // this works
  return function bar(defaultValue) {
    const myIndex = walker;
    walker += 1;

    if (arr.length < myIndex + 1) {
      arr[myIndex] = defaultValue;
    }
  }
}

const bar = foo();
bar(null);
console.log(arr); // [null]

The closure seems to necessary, moving walker to the outer scope also works in all cases.

Last but not least a playground link

[kdy1]

Seems like this issue is partially fixed - transform() + minify() works, but transform() with minify option is not fixed.
So... I'm not sure how should I reproduce this

[kdy1]

Oh I think this can be an issue of PatOrExpr issue

[swc-bot]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.