Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge branch in if #166

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

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 e87bacf
🔍 Latest deploy log https://app.netlify.com/sites/webcrack/deploys/67cd8a8e64636300089e9d9f
😎 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++;
           }

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