The latest updates on your projects. Learn more about Vercel for Git ↗︎

hm you might need to rebase to get latest changes

and please add corresponding test cases. Let me know if you need some help on this.

and please add corresponding test cases. Let me know if you need some help on this.

Sure, Thank you!

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.33%. Comparing base (8546f81) to head (f1677df).

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   89.31%   89.33%   +0.01%     
==========================================
  Files         100      100              
  Lines       12316    12337      +21     
  Branches     1635     1640       +5     
==========================================
+ Hits        11000    11021      +21     
  Misses       1259     1259              
  Partials       57       57              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I have added the test cases as per the code.

Sorry about so many commits, I didn't check the test case before.

hmm let me figure out how to rebase your branch

I just rebase my branch.

I just squash the changes. I will review this pr tomorrow 🙏

Hi, I just reviewed and squashed all the commits. Please jump to the latest before you add any commits. 🙏

I have to say sorry that I removed the ternary and array inlining. Here are the reasons:

1. The current Implementation will fail on a lot of edge cases. You can jump to the commit that I added test cases to see what's wrong.
2. Constant folding or let's say the "compression" in SWC/Terser is difficult. We need to do a lot of work to make it "correct". If we want to add this ability to wakaru, then it should be added as a standalone rule that inlines variables and constant results. It's not a JSX-specific thing.
3. Most modern compilers alr do the compression for you. So we can confidently say these are rare cases.

I just reviewed and squashed all the commits

@pionxzh As an aside, a better workflow for future may be squash commits when merging, rather than squashing a contributors working branch. I can't speak for all, but if a maintainer squashed and force pushed the branch I was working on, that could be enough to make me just abandon the PR all together.

@0xdevalias Thanks for the reminder. That's definitely not a good workflow.

@AhmadFaraz-crypto Sorry for what I did. This PR was stuck at a point where there were conflicts with the main branch. I was trying to resolve the conflicts, and I shouldn't squash the commits in the process. I will make sure to not do it again.

I'm away from my PC right now, but I want to talk more about why some of the changes were made. I will do that as soon as I get back.

1. There is a handy helper function findDeclaration that can be used to find the declaration of a variable in a given scope.
   If we simply use root.find(j.VariableDeclarator, ...) to find the declaration of a variable, it will not work in all cases, especially when the script is huge and there are multiple nested declarations.

2. The ternary inlining will fail in some cases where the condition is a complex expression or not defined because the code assumes that if the condition is not truthy, the alternate will be used. This is not always the case. This will be troublesome if we want to handle all edge cases.

- Expected
+ Received

React.createElement(true ? "a" : "div", null);
- <a />
+ <div />

function fn() {
  return React.createElement(
    r ? "a" : "div",
    null,
    "Hello",
  );
}

function fn() {
-  const Component = r ? "a" : "div";
-  return <Component>Hello</Component>;
+  return <div>Hello</div>;
}

3. This one is minor. We need to make sure that the inlined tag name is a valid JSX tag name. For example, the following code will fail.

  function fn() {
-  const Name = 1;
-  return <Name />;
+  return <1 />;
}

4. The array inlining will fail if the index is not a number or a valid identifier. For example, the following code will fail.

function fn() {
  const components = ["div", "a"]
  return React.createElement(
    components['99'],
    null,
    "Hello",
  );
}

function fn() {
-  return <div>Hello</div>;
+  const components = ["div", "a"]
+  return <components.99>Hello</components.99>;
}

These are some of the edge cases, and they are fixable. But like I said, these are rare cases that shouldn't be written by developers in the first place, modern toolchains will eliminate these cases also. The performance degradation brought by handling these cases is also not worth it. That's why I decided to remove them.

I should discuss the expectations with you first before making any changes. Or maybe, I should think about these cases and bring up the discussion in the first place when I receive the PR.

@pionxzh Thank you! That's ok. I just reviewed your code and it looks good. Yes it will be helpful if you explain all the scenarios before so I will implement logic accordingly. I think as per this ticket number #115 this MR should be closed. I should not work on the ternary and array implementations and I should discuss thing with you and I think I discussed array thing with you in previous comments.

I think you should create a separate ticket for inline ternary and array implementation with specification.

#115 still needs the props inlining to be considered "completed". But let's merge this first.