Skip to content

Conversation

Le0Developer
Copy link
Contributor

closes #165

not sure how I feel on the effect it had on the webpack test

Copy link

netlify bot commented Mar 9, 2025

Deploy Preview for webcrack ready!

Name Link
🔨 Latest commit 8343621
🔍 Latest deploy log https://app.netlify.com/sites/webcrack/deploys/67e533adecbfce0008cbdaef
😎 Deploy Preview https://deploy-preview-166--webcrack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@j4k0xb
Copy link
Owner

j4k0xb commented Mar 9, 2025

How about an additional flip-conditionals transform instead of modifying merge-else-if?

  • if (!test) { a } else { b } -> if (test) { b } else { a } (only when else exists, not for else if)
  • !test ? a : b -> test ? b : a

Would solve your original issue and a few others at the same time

nvm just tried this and it made some improvements but also resulted in similarly unreadable code like in the webpack test

@Le0Developer
Copy link
Contributor Author

not affect this webpack test

Done, but it still affects the webpack test because it's an else if.

improve readability (no ! or !!)

IMO this should be solved by another transformation which transforms things like !(a === b) to a !== b and !(typeof val == "boolean" || val == null) -> typeof val != "boolean" && val != null.

@Le0Developer
Copy link
Contributor Author

Fixed the unreadable webpack code by preventing the merge if it's already an else if

@j4k0xb
Copy link
Owner

j4k0xb commented Mar 9, 2025

IMO this should be solved by another transformation which transforms things like !(a === b)

Already exists: https://webcrack.netlify.app/docs/concepts/unminify.html#invert-boolean-logic
But what I meant would only apply to if-else and ternary

@j4k0xb
Copy link
Owner

j4k0xb commented Mar 9, 2025

image

Hmm not sure how to go about that... It successfully flattened it but due to the order of transforms (onExit, meaning merge-else-if will run after invert-boolean-logic) there's an extra !(...) now

And in other cases getting rid of ! wouldn't be possible:

// >, >=, <, <= are not invertible because NaN <= 0 is false and NaN > 0 is false
// https://tc39.es/ecma262/multipage/abstract-operations.html#sec-islessthan

image

@Le0Developer
Copy link
Contributor Author

There's more such cases, e.g. I have to re-run webcrack 4 times before it becomes stable.

We can explicitly call the invert-boolean-logic transform here like how control-flow-object is explicitly calling merge-strings.

// Example: _0x29d709["kHAOU"] = "5|1|2" + "|4|3|" + "0|6";
// For performance reasons, only traverse if it is a potential match (value doesn't matter)
if (looseAssignment.match(statement)) {
applyTransform(statement, mergeStrings);
}

@j4k0xb
Copy link
Owner

j4k0xb commented Mar 9, 2025

We can explicitly call the invert-boolean-logic transform

yes that's technically possible but have to be careful to avoid quadratic runtime (traverse AST while traversing AST) or infinite loops
only running it on the test expression could work

@Le0Developer
Copy link
Contributor Author

Le0Developer commented Mar 9, 2025

only running it on the test expression could work

Yeah that's what I meant, will push a commit for this in a bit.

Update: looks like it doesn't work for some reason. The test is failing but it shouldn't 🤷

diff --git a/packages/webcrack/src/unminify/test/merge-else-if.test.ts b/packages/webcrack/src/unminify/test/merge-else-if.test.ts
index 1a11585..887fa67 100644
--- a/packages/webcrack/src/unminify/test/merge-else-if.test.ts
+++ b/packages/webcrack/src/unminify/test/merge-else-if.test.ts
@@ -21,7 +21,7 @@ test('merge', () => {
     } else {
       console.log("branch 1");
     }`).toMatchInlineSnapshot(`
-      if (!!cond) {
+      if (cond) {
         console.log("branch 1");
       } else if (cond2) {
         console.log("branch 2");
diff --git a/packages/webcrack/src/unminify/transforms/merge-else-if.ts b/packages/webcrack/src/unminify/transforms/merge-else-if.ts
index c8d3b19..bfa76cf 100644
--- a/packages/webcrack/src/unminify/transforms/merge-else-if.ts
+++ b/packages/webcrack/src/unminify/transforms/merge-else-if.ts
@@ -1,6 +1,7 @@
 import * as m from '@codemod/matchers';
 import * as t from '@babel/types';
-import type { Transform } from '../../ast-utils';
+import { applyTransform, type Transform } from '../../ast-utils';
+import invertBooleanLogic from './invert-boolean-logic';

 export default {
   name: 'merge-else-if',
@@ -43,6 +44,10 @@ export default {
             path.node.test = t.unaryExpression('!', path.node.test);
             path.node.consequent = path.node.alternate!;
             path.node.alternate = nestedIf.current;
+            this.changes += applyTransform(
+              path.node.test,
+              invertBooleanLogic,
+            ).changes;
             this.changes++;
           }

@Le0Developer
Copy link
Contributor Author

Update 3 weeks later: I just realized why it didn't remove the !!; the transform doesn't know it's running in the conditional, so it thinks it could be a cast to bool. This transform is done by remove-double-not instead.

@Le0Developer
Copy link
Contributor Author

Update: if I don't call applyTransforms with path.node (instead of just path.node.test which would be more efficient), it just doesn't match the root UnaryExpression for some reason. 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

if/else flattening in if branch
2 participants