feat: remove unused import(...) if importee doesn't have side-effects#3911
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0c6e806 to
e1e4fe8
Compare
|
I just inline the |
e1e4fe8 to
1fee96f
Compare
1fee96f to
29551b0
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
29551b0 to
f25fb46
Compare
Benchmarks Rust
|
f25fb46 to
9ad4404
Compare
|
Please hold off on merging for now. I've encountered some special cases before, and I need to test them first. |
shulaoda
left a comment
shulaoda
left a comment
There was a problem hiding this comment.
It seems that the following scenario is not supported:
// entry.ts
async function test() {
const a = await import('./hello')
const b = await import('./hello')
console.log(a === b)
}
test()
// hello.tsrolldown:
// entry.js
async function test() {
const a = await import("./hello-hoTXCQ30.js");
const b = await import("./hello-hoTXCQ30.js");
console.log(a === b);
}
test();
// hello-hoTXCQ30.jsrollup:
async function test() {
const a = await Promise.resolve().then(function () { return hello; });
const b = await Promise.resolve().then(function () { return hello; });
console.log(a === b);
}
test();
var hello = /*#__PURE__*/Object.freeze({
__proto__: null
});
shulaoda
left a comment
shulaoda
left a comment
There was a problem hiding this comment.
The following scenario is also not supported:
// entry.ts
import('./hello')
// hello.ts
function loadTS() {
try {
return import('typescript').then((m) => m.default)
} catch (e) {
console.error('typescript must be installed to use "native" functions')
throw e
}
}rolldown:
// entry.js
import("./hello-DavaYWk6.js");
// hello-DavaYWk6.jsrollup:
Promise.resolve().then(function () { return hello; });
var hello = /*#__PURE__*/Object.freeze({
__proto__: null
});
There was a problem hiding this comment.
// entry.ts
import('./a')
import('./b')
// a.ts
import('./b')
// b.tsrolldown:
// entry.js
import("./a-h86hG9tv.js");
Promise.resolve().then(function() {
return /* @__PURE__ */ Object.freeze({ __proto__: null });
});
// a-h86hG9tv.js
Promise.resolve().then(function() {
return /* @__PURE__ */ Object.freeze({ __proto__: null });
});rollup:
// entry.js
import('./a-CHPDa8MY.js');
Promise.resolve().then(function () { return b; });
var b = /*#__PURE__*/Object.freeze({
__proto__: null
});
export { b };
// a-CHPDa8MY.js
import('./main.js').then(function (n) { return n.b; });
|
Our analyzer only analyzes top level stmt, so it is not our goal to support these patterns. |
What do you mean by this example? Can you give some specific code rather than pseudo code? |
As I said I don't find it very useful to declare an extra variable, |
|
I will try to support these pattern in next pr. |
What's inside |
empty file |
I'm wondering if there are users who store the |
|
I'm just listing some cases I encountered during my implementation attempts, more scenarios may need to be considered. |
For this case, both of rollup and rolldown generate two chunks, rollup will try generate an extra export symbol, I am not sure if it is a good strategy to export symbol during bundling, or chunk splitting only care about user defined export symbol. |
Can you give an example? If the import stmt is store as an variable which means it is used, current impl will not generate |
Will try to align with rollup but it is hard to align with them in all scenario because our chunk splitting impl in different way. |
For this case it because |
…ts in nested scope refs #3911
3 participants
Description
import(...)if importee doesn't have side-effects #2827, closed feat(rust): remove unusedimport(..)if importee doesn't have side-effects #3598console.logor some module in tests will be broken since the dynamic module is side effects free.