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.

Sign up for GitHub

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

Jump to bottom

fix(es/minifier): Make AST compressor respect toplevel #6775

Merged
merged 15 commits into from
Jan 11, 2023
Merged

fix(es/minifier): Make AST compressor respect toplevel #6775

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2725f0d
Move
kdy1 Jan 10, 2023
f959f7b
Add tests
kdy1 Jan 10, 2023
67b240a
Update test refs
kdy1 Jan 10, 2023
bc3b505
Check top level
kdy1 Jan 10, 2023
49a75f0
Fix
kdy1 Jan 10, 2023
d15c922
Update test refs
kdy1 Jan 10, 2023
0a033c6
config.json
kdy1 Jan 10, 2023
957bc15
Fix
kdy1 Jan 11, 2023
41c502f
Update test refs
kdy1 Jan 11, 2023
aa11c37
Remove comments
kdy1 Jan 11, 2023
ed25fe6
Reduce input
kdy1 Jan 11, 2023
e782490
Update test refs
kdy1 Jan 11, 2023
2793655
Move down
kdy1 Jan 11, 2023
6e64e03
Merge branch 'main' into minifier-top-unused
swc-bot Jan 11, 2023
099720d
Update test refs
kdy1 Jan 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/swc_ecma_minifier/src/compress/optimize/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ where
&mut self,
ident: &mut Ident,
init: &mut Expr,
should_preserve: bool,
mut should_preserve: bool,
can_drop: bool,
) {
trace_op!(
Expand Down Expand Up @@ -58,6 +58,9 @@ where
if !usage.var_initialized {
return;
}

should_preserve |= !self.options.top_level() && usage.is_top_level;
kdy1 marked this conversation as resolved.
Show resolved Hide resolved

if self.data.top.used_arguments && usage.declared_as_fn_param {
return;
}
Expand Down
5 changes: 5 additions & 0 deletions crates/swc_ecma_minifier/src/program_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ pub(crate) struct VarUsageInfo {

pub(crate) pure_fn: bool,

/// Is the variable declared in top level?
pub(crate) is_top_level: bool,

/// `infects_to`. This should be renamed, but it will be done with another
/// PR. (because it's hard to review)
infects: Vec<Access>,
Expand Down Expand Up @@ -168,6 +171,7 @@ impl Default for VarUsageInfo {
used_in_non_child_fn: Default::default(),
accessed_props: Default::default(),
used_recursively: Default::default(),
is_top_level: Default::default(),
}
}
}
Expand Down Expand Up @@ -347,6 +351,7 @@ impl Storage for ProgramData {
// }

let v = self.vars.entry(i.to_id()).or_default();
v.is_top_level |= ctx.is_top_level;

if has_init && (v.declared || v.var_initialized) {
#[cfg(feature = "debug")]
Expand Down
4 changes: 4 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/1/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"defaults": true,
"toplevel": false
}
57 changes: 57 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/1/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
var application;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll reduce this file

/******/ (() => { // webpackBootstrap/******/ "use strict";
/******/ // The require scope
/******/ var __webpack_require__ = {};
/******/
/************************************************************************/
/******/ /* webpack/runtime/define property getters */
/******/ (() => {
/******/ // define getter functions for harmony exports
/******/ __webpack_require__.d = (exports, definition) => {
/******/ for (var key in definition) {
/******/ if (__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) {
/******/ Object.defineProperty(exports, key, { enumerable: true, get: definition[key] });
/******/
}
/******/
}
/******/
};
/******/
})();
/******/
/******/ /* webpack/runtime/hasOwnProperty shorthand */
/******/ (() => {
/******/ __webpack_require__.o = (obj, prop) => (Object.prototype.hasOwnProperty.call(obj, prop))
/******/
})();
/******/
/******/ /* webpack/runtime/make namespace object */
/******/ (() => {
/******/ // define __esModule on exports
/******/ __webpack_require__.r = (exports) => {
/******/ if (typeof Symbol !== 'undefined' && Symbol.toStringTag) {
/******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
/******/
}
/******/ Object.defineProperty(exports, '__esModule', { value: true });
/******/
};
/******/
})();
/******/
/************************************************************************/
var __webpack_exports__ = {};
__webpack_require__.r(__webpack_exports__);
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
/* harmony export */ "bootstrap": () => (/* binding */ bootstrap)
/* harmony export */
});
function bootstrap() {
alert();
}

application = __webpack_exports__;
/******/
})()
;
23 changes: 23 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/1/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
var application;
(()=>{
var __webpack_require__ = {};
__webpack_require__.d = (exports, definition)=>{
for(var key in definition)__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key) && Object.defineProperty(exports, key, {
enumerable: !0,
get: definition[key]
});
}, __webpack_require__.o = (obj, prop)=>Object.prototype.hasOwnProperty.call(obj, prop), __webpack_require__.r = (exports)=>{
'undefined' != typeof Symbol && Symbol.toStringTag && Object.defineProperty(exports, Symbol.toStringTag, {
value: 'Module'
}), Object.defineProperty(exports, '__esModule', {
value: !0
});
};
var __webpack_exports__ = {};
function bootstrap() {
alert();
}
__webpack_require__.r(__webpack_exports__), __webpack_require__.d(__webpack_exports__, {
bootstrap: ()=>bootstrap
}), application = __webpack_exports__;
})();
4 changes: 4 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/2/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"defaults": true,
"toplevel": true
}
57 changes: 57 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/2/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
var application;
/******/ (() => { // webpackBootstrap/******/ "use strict";
/******/ // The require scope
/******/ var __webpack_require__ = {};
/******/
/************************************************************************/
/******/ /* webpack/runtime/define property getters */
/******/ (() => {
/******/ // define getter functions for harmony exports
/******/ __webpack_require__.d = (exports, definition) => {
/******/ for (var key in definition) {
/******/ if (__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key)) {
/******/ Object.defineProperty(exports, key, { enumerable: true, get: definition[key] });
/******/
}
/******/
}
/******/
};
/******/
})();
/******/
/******/ /* webpack/runtime/hasOwnProperty shorthand */
/******/ (() => {
/******/ __webpack_require__.o = (obj, prop) => (Object.prototype.hasOwnProperty.call(obj, prop))
/******/
})();
/******/
/******/ /* webpack/runtime/make namespace object */
/******/ (() => {
/******/ // define __esModule on exports
/******/ __webpack_require__.r = (exports) => {
/******/ if (typeof Symbol !== 'undefined' && Symbol.toStringTag) {
/******/ Object.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });
/******/
}
/******/ Object.defineProperty(exports, '__esModule', { value: true });
/******/
};
/******/
})();
/******/
/************************************************************************/
var __webpack_exports__ = {};
__webpack_require__.r(__webpack_exports__);
/* harmony export */ __webpack_require__.d(__webpack_exports__, {
/* harmony export */ "bootstrap": () => (/* binding */ bootstrap)
/* harmony export */
});
function bootstrap() {
alert();
}

application = __webpack_exports__;
/******/
})()
;
22 changes: 22 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/4386/2/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
(()=>{
var __webpack_require__ = {};
__webpack_require__.d = (exports, definition)=>{
for(var key in definition)__webpack_require__.o(definition, key) && !__webpack_require__.o(exports, key) && Object.defineProperty(exports, key, {
enumerable: !0,
get: definition[key]
});
}, __webpack_require__.o = (obj, prop)=>Object.prototype.hasOwnProperty.call(obj, prop), __webpack_require__.r = (exports)=>{
'undefined' != typeof Symbol && Symbol.toStringTag && Object.defineProperty(exports, Symbol.toStringTag, {
value: 'Module'
}), Object.defineProperty(exports, '__esModule', {
value: !0
});
};
var __webpack_exports__ = {};
function bootstrap() {
alert();
}
__webpack_require__.r(__webpack_exports__), __webpack_require__.d(__webpack_exports__, {
bootstrap: ()=>bootstrap
});
})();
4 changes: 4 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/projects/next/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"defaults": true,
"toplevel": true
}
4 changes: 2 additions & 2 deletions ..._ecma_minifier/tests/terser/compress/collapse_vars/side_effect_free_replacement/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
var b;
x(b);
var b, a;
a = b, x(a);
8 changes: 8 additions & 0 deletions crates/swc_ecma_transforms_optimization/src/simplify/dce/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,14 @@ impl TreeShaker {
} else if !self.in_block_stmt {
return false;
}

// Abort if the variable is declared on top level scope.
let ix = self.data.graph_ix.get_index_of(&name);
if let Some(ix) = ix {
if self.data.entries.contains(&(ix as u32)) {
return false;
}
}
}

if self.config.top_retain.contains(&name.0) {
Expand Down
3 changes: 3 additions & 0 deletions crates/swc_ecma_usage_analyzer/src/analyzer/ctx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ where
}

#[derive(Debug, Default, Clone, Copy)]
#[non_exhaustive]
pub struct Ctx {
/// See [crate::marks::Marks]
pub skip_standalone: bool,
Expand Down Expand Up @@ -60,6 +61,8 @@ pub struct Ctx {
pub inline_prevented: bool,

pub is_op_assign: bool,

pub is_top_level: bool,
}

pub(super) struct WithCtx<'a, S>
Expand Down
7 changes: 6 additions & 1 deletion crates/swc_ecma_usage_analyzer/src/analyzer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ where
let mut child = UsageAnalyzer {
data: Default::default(),
marks: self.marks,
ctx: self.ctx,
ctx: Ctx {
is_top_level: false,
..self.ctx
},
scope: Default::default(),
used_recursively: self.used_recursively.clone(),
};
Expand Down Expand Up @@ -849,6 +852,7 @@ where
fn visit_module(&mut self, n: &Module) {
let ctx = Ctx {
skip_standalone: true,
is_top_level: true,
..self.ctx
};
n.visit_children_with(&mut *self.with_ctx(ctx))
Expand All @@ -857,6 +861,7 @@ where
fn visit_script(&mut self, n: &Script) {
let ctx = Ctx {
skip_standalone: true,
is_top_level: true,
..self.ctx
};
n.visit_children_with(&mut *self.with_ctx(ctx))
Expand Down