-
Notifications
You must be signed in to change notification settings - Fork 73
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
fix: hash not stable (&revert import * as optimize) #1610
Changes from 8 commits
c05fb46
198dff8
bea938c
19bfccd
e01bcc3
b4335d1
c0b9e5e
1993386
58e7167
7447872
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ use crate::compiler::Context; | |
use crate::module_graph::ModuleGraph; | ||
use crate::plugin::{Plugin, PluginTransformJsParam}; | ||
|
||
mod collect_explicit_prop; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 建议移除未使用的模块
|
||
mod module; | ||
mod module_side_effects_flag; | ||
mod remove_useless_stmts; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,222 @@ | ||
use std::collections::{HashMap, HashSet}; | ||
|
||
use swc_core::ecma::ast::{ComputedPropName, Id, Ident, Lit, MemberExpr, MemberProp}; | ||
use swc_core::ecma::visit::{Visit, VisitWith}; | ||
|
||
#[derive(Debug)] | ||
pub struct IdExplicitPropAccessCollector { | ||
to_detected: HashSet<Id>, | ||
accessed_by_explicit_prop_count: HashMap<Id, usize>, | ||
ident_accessed_count: HashMap<Id, usize>, | ||
accessed_by: HashMap<Id, HashSet<String>>, | ||
} | ||
|
||
impl IdExplicitPropAccessCollector { | ||
pub(crate) fn new(ids: HashSet<Id>) -> Self { | ||
Self { | ||
to_detected: ids, | ||
accessed_by_explicit_prop_count: Default::default(), | ||
ident_accessed_count: Default::default(), | ||
accessed_by: Default::default(), | ||
} | ||
} | ||
pub(crate) fn explicit_accessed_props(mut self) -> HashMap<String, Vec<String>> { | ||
self.to_detected | ||
.iter() | ||
.filter_map(|id| { | ||
let member_prop_accessed = self.accessed_by_explicit_prop_count.get(id); | ||
let ident_accessed = self.ident_accessed_count.get(id); | ||
|
||
match (member_prop_accessed, ident_accessed) { | ||
// all ident are accessed explicitly, so there is member expr there is a name | ||
// ident, and at last plus the extra ident in import decl, that's 1 comes from. | ||
(Some(m), Some(i)) if (i - m) == 1 => { | ||
let mut accessed_by = Vec::from_iter(self.accessed_by.remove(id).unwrap()); | ||
accessed_by.sort(); | ||
|
||
let str_key = format!("{}#{}", id.0, id.1.as_u32()); | ||
|
||
Some((str_key, accessed_by)) | ||
} | ||
// Some un-explicitly access e.g: obj[foo] | ||
_ => None, | ||
} | ||
}) | ||
.collect() | ||
} | ||
|
||
fn increase_explicit_prop_accessed_count(&mut self, id: Id) { | ||
self.accessed_by_explicit_prop_count | ||
.entry(id.clone()) | ||
.and_modify(|c| { | ||
*c += 1; | ||
}) | ||
.or_insert(1); | ||
} | ||
|
||
fn insert_member_accessed_by(&mut self, id: Id, prop: &str) { | ||
self.increase_explicit_prop_accessed_count(id.clone()); | ||
self.accessed_by | ||
.entry(id) | ||
.and_modify(|accessed| { | ||
accessed.insert(prop.to_string()); | ||
}) | ||
.or_insert(HashSet::from([prop.to_string()])); | ||
} | ||
} | ||
|
||
impl Visit for IdExplicitPropAccessCollector { | ||
fn visit_ident(&mut self, n: &Ident) { | ||
let id = n.to_id(); | ||
|
||
if self.to_detected.contains(&id) { | ||
self.ident_accessed_count | ||
.entry(id) | ||
.and_modify(|c| { | ||
*c += 1; | ||
}) | ||
.or_insert(1); | ||
} | ||
} | ||
|
||
fn visit_member_expr(&mut self, n: &MemberExpr) { | ||
if let Some(obj_ident) = n.obj.as_ident() { | ||
let id = obj_ident.to_id(); | ||
|
||
if self.to_detected.contains(&id) { | ||
match &n.prop { | ||
MemberProp::Ident(prop_ident) => { | ||
self.insert_member_accessed_by(id, prop_ident.sym.as_ref()); | ||
} | ||
MemberProp::PrivateName(_) => {} | ||
MemberProp::Computed(ComputedPropName { expr, .. }) => { | ||
if let Some(lit) = expr.as_lit() | ||
&& let Lit::Str(str) = lit | ||
{ | ||
let visited_by = str.value.to_string(); | ||
self.insert_member_accessed_by(id, &visited_by) | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
n.visit_children_with(self); | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use maplit::hashset; | ||
|
||
use super::*; | ||
use crate::ast::tests::TestUtils; | ||
|
||
#[test] | ||
fn test_no_prop() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
console.log(foo) | ||
"#, | ||
); | ||
|
||
assert_eq!(fields, None); | ||
} | ||
#[test] | ||
fn test_no_access() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
"#, | ||
); | ||
|
||
assert_eq!(fields, None); | ||
} | ||
|
||
#[test] | ||
fn test_computed_prop() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
foo['f' + 'o' + 'o'] | ||
"#, | ||
); | ||
|
||
assert_eq!(fields, None); | ||
} | ||
|
||
#[test] | ||
fn test_simple_explicit_prop() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
foo.x; | ||
foo.y; | ||
"#, | ||
); | ||
|
||
assert_eq!(fields.unwrap(), vec!["x".to_string(), "y".to_string()]); | ||
} | ||
|
||
#[test] | ||
fn test_nest_prop_explicit_prop() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
foo.x.z[foo.y] | ||
"#, | ||
); | ||
|
||
assert_eq!(fields.unwrap(), vec!["x".to_string(), "y".to_string()]); | ||
} | ||
|
||
#[test] | ||
fn test_string_literal_prop_explicit() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
foo['x'] | ||
"#, | ||
); | ||
|
||
assert_eq!(fields.unwrap(), vec!["x".to_string()]); | ||
} | ||
|
||
#[test] | ||
fn test_num_literal_prop_not_explicit() { | ||
let fields = extract_explicit_fields( | ||
r#" | ||
import * as foo from "./foo.js"; | ||
foo[1] | ||
"#, | ||
); | ||
|
||
assert_eq!(fields, None); | ||
} | ||
|
||
fn extract_explicit_fields(code: &str) -> Option<Vec<String>> { | ||
let tu = TestUtils::gen_js_ast(code); | ||
|
||
let id = namespace_id(&tu); | ||
let str = format!("{}#{}", id.0, id.1.as_u32()); | ||
|
||
let mut v = IdExplicitPropAccessCollector::new(hashset! { id }); | ||
tu.ast.js().ast.visit_with(&mut v); | ||
|
||
v.explicit_accessed_props().remove(&str) | ||
} | ||
|
||
fn namespace_id(tu: &TestUtils) -> Id { | ||
tu.ast.js().ast.body[0] | ||
.as_module_decl() | ||
.unwrap() | ||
.as_import() | ||
.unwrap() | ||
.specifiers[0] | ||
.as_namespace() | ||
.unwrap() | ||
.local | ||
.to_id() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,16 @@ | ||
use std::collections::HashSet; | ||
|
||
use swc_core::common::util::take::Take; | ||
use swc_core::common::SyntaxContext; | ||
use swc_core::ecma::ast::{ | ||
Decl, ExportDecl, ExportSpecifier, ImportDecl, ImportSpecifier, Module as SwcModule, | ||
ModuleExportName, | ||
Decl, ExportDecl, ExportSpecifier, Id, ImportDecl, ImportSpecifier, Module as SwcModule, | ||
Module, ModuleExportName, | ||
}; | ||
use swc_core::ecma::transforms::compat::es2015::destructuring; | ||
use swc_core::ecma::transforms::compat::es2018::object_rest_spread; | ||
use swc_core::ecma::visit::{VisitMut, VisitMutWith, VisitWith}; | ||
|
||
use super::collect_explicit_prop::IdExplicitPropAccessCollector; | ||
use crate::plugins::tree_shaking::module::TreeShakeModule; | ||
use crate::plugins::tree_shaking::statement_graph::analyze_imports_and_exports::{ | ||
analyze_imports_and_exports, StatementInfo, | ||
|
@@ -105,10 +112,10 @@ | |
|
||
// remove from the end to the start | ||
stmts_to_remove.reverse(); | ||
|
||
for stmt in stmts_to_remove { | ||
swc_module.body.remove(stmt); | ||
} | ||
optimize_import_namespace(&mut used_import_infos, swc_module); | ||
|
||
(used_import_infos, used_export_from_infos) | ||
} | ||
|
@@ -231,6 +238,72 @@ | |
} | ||
} | ||
|
||
fn optimize_import_namespace(import_infos: &mut [ImportInfo], module: &mut Module) { | ||
let namespaces = import_infos | ||
.iter() | ||
.filter_map(|import_info| { | ||
let ns = import_info | ||
.specifiers | ||
.iter() | ||
.filter_map(|sp| match sp { | ||
ImportSpecifierInfo::Namespace(ns) => Some(ns.clone()), | ||
_ => None, | ||
}) | ||
.collect::<Vec<String>>(); | ||
if ns.is_empty() { | ||
None | ||
} else { | ||
Some(ns) | ||
} | ||
}) | ||
.flatten() | ||
.collect::<Vec<String>>(); | ||
|
||
let ids = namespaces | ||
.iter() | ||
.map(|ns| { | ||
let (sym, ctxt) = ns.rsplit_once('#').unwrap(); | ||
(sym.into(), SyntaxContext::from_u32(ctxt.parse().unwrap())) | ||
Comment on lines
+265
to
+266
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 避免使用 在第265行和266行,代码直接使用了 您可以修改代码如下: - let (sym, ctxt) = ns.rsplit_once('#').unwrap();
- (sym.into(), SyntaxContext::from_u32(ctxt.parse().unwrap()))
+ if let Some((sym, ctxt_str)) = ns.rsplit_once('#') {
+ if let Ok(ctxt_num) = ctxt_str.parse() {
+ (sym.into(), SyntaxContext::from_u32(ctxt_num))
+ } else {
+ // 处理解析失败的情况
+ }
+ } else {
+ // 处理未找到 '#' 的情况
+ }
|
||
}) | ||
.collect::<HashSet<Id>>(); | ||
|
||
if !ids.is_empty() { | ||
let mut v = IdExplicitPropAccessCollector::new(ids); | ||
let mut shadow = module.clone(); | ||
|
||
Comment on lines
+272
to
+273
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 优化性能:避免克隆整个模块 在第272行,代码克隆了整个模块 |
||
shadow.visit_mut_with(&mut object_rest_spread(Default::default())); | ||
shadow.visit_mut_with(&mut destructuring(Default::default())); | ||
shadow.visit_with(&mut v); | ||
|
||
let explicit_prop_accessed_ids = v.explicit_accessed_props(); | ||
|
||
import_infos.iter_mut().for_each(|ii| { | ||
ii.specifiers = ii | ||
.specifiers | ||
.take() | ||
.into_iter() | ||
.flat_map(|specifier_info| { | ||
if let ImportSpecifierInfo::Namespace(ref ns) = specifier_info { | ||
if let Some(visited_fields) = explicit_prop_accessed_ids.get(ns) { | ||
return visited_fields | ||
.iter() | ||
.map(|v| { | ||
let imported_name = format!("{v}#0"); | ||
ImportSpecifierInfo::Named { | ||
imported: Some(imported_name.clone()), | ||
local: imported_name, | ||
} | ||
}) | ||
.collect::<Vec<_>>(); | ||
} | ||
} | ||
vec![specifier_info] | ||
}) | ||
.collect::<Vec<_>>(); | ||
}) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在迭代过程中修改图结构可能导致未定义行为
在
rewrite_dependency
方法中,您在迭代edges
的同时删除边,这可能导致迭代器失效和未定义行为。建议在迭代之前先收集所有需要删除的边的索引,然后再进行删除操作。建议修改代码如下: