feat(es/minifier): Handle more indexing expression

[levi-nz]

Description:

This PR is a WIP that fixes all known issues in #8747.

Related issue:

• Closes expr_simplifier does not simplify computed MemberExprs with non-literal properties #8747

[changeset-bot]

⚠️ No Changeset found

Latest commit: cc2dcab

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[levi-nz]

Not totally sure if the changed code for array literals is correct -- the code for this is a little more complicated than strings.

[][0] works correctly for now, but not [][[]]. Will look into this later.

[levi-nz]

Another broken test case:

var _ = {0.5: 'test'}[0.5];

Currently simplifies to:

var _ = {
    0.5: 'test'
}[0.5];

Should simplify to:

var _ = 'test';

Not entirely sure if this is correct though because removing the ObjectLit could have side effects, ie if a value is a CallExpr

[levi-nz]

Only one potential issue so far, that KnownOp no longer implements Eq because of Index now taking f64 instead of i64, but seems OK so far to me.

[levi-nz]

@kdy1 The only remaining issue in terms of code changes is this line. Currently is_literal(props) does not work with the unit test below because the 0.5 is not considered a literal. I'm not sure why this fract() check exists and I'm not sure if changing this would break something.

https://github.com/levi-nz/swc/blob/454378d7eaac82bae40c5a770888e84580a1381b/crates/swc_ecma_utils/src/lib.rs#L1883

fold("({0.5: 'a'})['0.5']", "'a';");

[levi-nz]

I'm pretty sure Index(f64) can be changed back to Index(i64) as well, as IndexStr is always used for objects.

[levi-nz]

I have ran UPDATE=1 cargo test --test projects --test tsc, but not sure if all of these files should be changed, especially crates/swc/tests/tsc-references/templateStringInIndexExpressionES6.2.minified.js, which is now empty.

[kdy1]

CI failed

[levi-nz]

This is getting a little complicated with property names, since we need to either correctly handle stuff like [].push not being replaced, or simply not replace invalid properties at all.

The solution I've implemented works and I don't think there's any issues, besides the fact that code like this will break:

Object.prototype.foobar = 5;
var _ = ({}).foobar; // returns 5, but will return undefined from the minifier

If there's no way to account for this correctly, we should ideally make this a configurable option where the user can make the minifier replace stuff like [].invalid with void 0 or just make it not replace anything at all.

If we continue with this approach, should we include deprecated properties like Object.watch to avoid any confusion (currently replaced with void 0), even though these no longer exist? This exists in vue.js. Ref: https://stackoverflow.com/questions/1029241/object-watch-for-all-browsers

In my opinion, replacing invalid properties needs to be a configurable option. In my use case, I want to replace invalid properties and ignore potential side effects, but this will introduce breaking changes for larger scripts and bundled scripts where other scripts are potentially changing the prototype of String, Object or Array. I assume there's no way to detect if the prototype of these are being changed, so it should just be an option that can be enabled/disabled. The whole reason this feature is implemented is so expressions like [][[]] can be minified, which is what I need for my use case.

[levi-nz]

Babel's minify doesn't modify expressions like [][[]], but Babel's path.evaluate() API lets you transform these expressions. Should we perhaps make this some sort of API instead? Not sure where to go from here, but this should definitely be an option to make the simplifier as powerful as Babel's path.evaluate() option.

[kdy1]

Please move logics to https://github.com/swc-project/swc/blob/8eac0bec49f4f260a261d87f174ed2913c4bee63/crates/swc_ecma_minifier/src/compress/pure/evaluate.rs

swc_ecma_utils is not a good place for this kind of optimizations

[levi-nz]

Please move logics to https://github.com/swc-project/swc/blob/8eac0bec49f4f260a261d87f174ed2913c4bee63/crates/swc_ecma_minifier/src/compress/pure/evaluate.rs

swc_ecma_utils is not a good place for this kind of optimizations

Do we not care about modified prototypes? For example, in the current release version:

swc/crates/swc_ecma_transforms_optimization/src/simplify/expr/mod.rs

Line 165 in 56e03a1

*expr = *undefined(*span)

We replace x[-1] with undefined, but that would break this code if minified:

Array.prototype[-1] = "foo";
[][-1] // outputs "foo", but swc replaces with void 0

Shouldn't this logic be moved to compress as well?

[kdy1]

There's an option named pristine_globals to mark globals as pristine, but I didn't bother to follow it in all places. Some rules uses it, but not all.

[levi-nz]

Got it, I will update the minifier accordingly to try use it -- I think that option should be checked here in the aforementioned case

[levi-nz]

Not sure why this exists, perhaps a bug with side effects?

swc/crates/swc_ecma_transforms_optimization/src/simplify/expr/mod.rs

Line 258 in 56e03a1

*expr = Expr::Seq(SeqExpr {

Current test case fold("[1,2,3,4,5,6,7,8,9,10][4 + []]", "5;"); fails, resulting in 0, 5 instead of just 5, removing that if block works but I think this may break things. Looking into why this is happening

[levi-nz]

I seem to have fixed this but not sure if it will cause side effects anywhere, will try take a look. It appears as ArrayLiterals being replaced were always being replaced with a SeqExpr and if no side effects existed then there was a dummy 0 being added to the front. It just gets replaced with the single Expr now instead.

[levi-nz]

Also, CI currently fails because of this. Is this normal? These unit tests don't seem to work on Windows, will try this on Mac later.

---- fixture_tests__terser__compress__evaluate__unsafe_string_bad_index__input_js stdout ----
Input: /home/runner/work/swc/swc/crates/swc_ecma_minifier/tests/terser/compress/evaluate/unsafe_string_bad_index/input.js
---- Config -----
{
    "evaluate": true,
    "unsafe": true
}

---- Input -----
console.log("1234".a + 1, "1234"["a"] + 1, "1234"[3.14] + 1);

thread 'fixture_tests__terser__compress__evaluate__unsafe_string_bad_index__input_js' panicked at crates/swc_ecma_minifier/src/compress/mod.rs:170:17:
Infinite loop detected (current pass = 202)
Code    0:









console.log("1234".a + 1, "1234"["a"] + 1, "1234"[3.14] + 1);


note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    fixture_tests__terser__compress__evaluate__unsafe_string_bad_index__input_js

test result: FAILED. 2674 passed; 1 failed; 27 ignored; 0 measured; 0 filtered out; finished in 96.08s

error: test failed, to rerun pass `-p swc_ecma_minifier --test compress`
Error: Process completed with exit code 101.

[levi-nz]

Not sure if I got the span stuff correct. There is some copy pasta going on here from simplify::expr, should probably move that stuff into their own util functions.

Where should I create unit tests for the new compress functionality? I assume in crates/swc_ecma_minifier/tests/compress.rs but there's no unit tests in there currently, so I'm unsure.

[kdy1]

You can create a directory and place an input.js and config.json in https://github.com/swc-project/swc/tree/56e03a19602b36a7ebac9ccc899065101bb385af/crates/swc_ecma_minifier/tests/fixture

Note that you need to touch (or save again) tests/compress.rs once after adding a test file

[levi-nz]

Will add watch and unwatch as exclusions for Object, even though they don't exist anymore. This way users can still use pristine_globals=true when using scripts like this to polyfill: https://gist.github.com/eligrey/384583

Not sure if there's any other exclusions we should add for property/method polyfills that don't exist anymore

[levi-nz]

Had to manually fix some files as rebasing messed a few up, hope that's OK

[kdy1]

Thank you!

---

swc-bump:

• swc_ecma_utils
• swc_ecma_minifier
• swc_ecma_transforms_optimization
• swc_core

[kdy1]

Thank you!

---

swc-bump:

• swc_ecma_utils
• swc_ecma_minifier
• swc_ecma_transforms_optimization
• swc_core