[regression] Side-effect-free Array.filter() prevents unused const from being removed

[leeoniya]

i keep rollup updated, so this regressed pretty recently.

• Rollup Version: 2.50.1 and REPL
• Operating System (or Browser): Linux
• Node Version (if applicable): 16.2
• Link to reproduction (IMPORTANT, read below):

Input

const foo = [1,2,3];

const bar = foo.filter(v => v % 1 == 0);

console.log(foo);

Expected Behavior

const foo = [1,2,3];

console.log(foo);

Actual Behavior

const foo = [1,2,3];

foo.filter(v => v % 1 == 0);

console.log(foo);

thanks!

[lukastaegert]

This is unfortunately not easily avoidable at the moment because we do not track if you mutate individual array elements via the filtered array. This was a bug in the previous implementation. While of course this is not a problem for an array of numbers, it would be a problem for other array elements, e.g.

const foo = [{value: false}, {value: false}];
const bar = foo.filter((v, i) => i % 1 == 0);
for (const element of bar) {
  bar.value = true;
}
console.log(foo);

Here you cannot remove the filter expression because that would change the result.

[lukastaegert]

So the current approach is to just assume that using array.filter mutates all array elements.

[leeoniya]

but in your example, bar is being used in the loop, so const bar = foo.filter((v, i) => i % 1 == 0); wouldn't get dropped due to that.

[lukastaegert]

Yes, I am just saying it is hard to track this without making mistakes. This would require a thorough rework of the Array builtin method handling logic and someone needs to do that. There is no one-line quick fix.

[leeoniya]

ok, thanks!

[lukastaegert]

Ok, I should not reply to thinks late in the evening. There is at least a partial improvement possible, see #4103. This will not fix your example, but the reason here is that passing the array to console.log currently deoptimizes the array fully, i.e. it assumes the filter method may be overwritten, and Rollup still cannot track side effect order.

[leeoniya]

Ok, I should not reply to thinks late in the evening.

no worries, guilty of this myself on a regular basis.

There is at least a partial improvement possible, see #4103.

this is the actual unused const that i had to remove manually that rollup removed previously: leeoniya/uPlot@cc28fd0

as you mentioned, #4103 doesn't have an effect on this.