feat: optimize in checks on namespaces to keep them treeshake-able
#6029
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Ah, the signature of traceExport had changed on master. I pushed a simple fix. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6029 +/- ##
=========================================
Coverage ? 98.75%
=========================================
Files ? 271
Lines ? 10597
Branches ? 2828
=========================================
Hits ? 10465
Misses ? 91
Partials ? 41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This PR has been released as part of rollup@4.46.0. You can test it via |
FWIW, the first iteration of my patch inlined all binary expressions that were statically resolved by Rollup; but it quickly turned out to be a potential footgun where the pre-computed result was longer than the binary operation itself. As it's not trivial to determine whether inlining is a win or not, I figured letting a code minifier tool deal with those was the best option for now and focus on making good use of Rollup's additional knowledge to do things Terser (and other minifiers) can't do. // This should not be inlined
var a = 'this is a rather long string that we really do not want to repeat multiple times'
var b = a + '.'
var c = a + '!' |
This PR contains:
Are tests included?
Breaking Changes?
Description
Currently, using the
inoperator on a namespace completely disables treeshaking, even if the left hand-side of the expression can be statically resolved. This makes checking for the existence of an export annoying, as demonstrated here.This PR optimizes these checks to collapse the expression to a boolean, that is also used for DCE which allows getting rid of eventual compat code and what not in libraries.
There is still a slight annoyance with how warnings are emitted, as shown by the added test. Despite the gate Rollup still emits a warning about the unresolved export. This is minor and I felt like it'd require more invasive changes, so I didn't address this here.