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.

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(plugin:emotion): panic when target to chrome 40 #1527

Merged
merged 7 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 2 additions & 21 deletions crates/mako/src/ast/js_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use crate::ast::{error, utils};
use crate::compiler::Context;
use crate::config::{DevtoolConfig, Mode, OutputMode};
use crate::module::Dependency;
use crate::plugin::PluginTransformJsParam;
use crate::utils::base64_encode;
use crate::visitors::dep_analyzer::DepAnalyzer;

Expand Down Expand Up @@ -134,7 +133,6 @@ impl JsAst {
&mut self,
mut_visitors: &mut Vec<Box<dyn visit::VisitMut>>,
folders: &mut Vec<Box<dyn visit::Fold>>,
file: &File,
should_inject_helpers: bool,
context: Arc<Context>,
) -> Result<()> {
Expand All @@ -151,28 +149,11 @@ impl JsAst {
}

// folders
let body = ast.body.take();
let mut module = Module {
span: ast.span,
shebang: ast.shebang.clone(),
body,
};
let mut module = ast.take();
for folder in folders {
module = folder.as_mut().fold_module(module);
}
ast.body = module.body;

// transform with plugin
context.plugin_driver.transform_js(
&PluginTransformJsParam {
handler,
path: file.path.to_str().unwrap(),
top_level_mark: self.top_level_mark,
unresolved_mark: self.unresolved_mark,
},
ast,
&context,
)?;
*ast = module;
Copy link
Contributor

Choose a reason for hiding this comment

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

优化 AST 转换逻辑

通过直接使用 ast.take() 方法创建 module,简化了控制流,减少了对 AST 的操作步骤。这种简化有助于提高代码的可读性和性能。


// FIXME: remove this, it's special logic
// inject helpers
Expand Down
88 changes: 58 additions & 30 deletions crates/mako/src/build/transform.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
use std::sync::Arc;

use anyhow::Result;
use swc_core::common::errors::HANDLER;
use swc_core::common::GLOBALS;
use swc_core::css::ast::{AtRule, AtRulePrelude, ImportHref, Rule, Str, Stylesheet, UrlValue};
use swc_core::css::compat::compiler::{self, Compiler};
use swc_core::css::{compat as swc_css_compat, prefixer, visit as swc_css_visit};
use swc_core::ecma::preset_env::{self as swc_preset_env};
use swc_core::ecma::transforms::base::feature::FeatureFlag;
use swc_core::ecma::transforms::base::fixer::paren_remover;
use swc_core::ecma::transforms::base::helpers::{Helpers, HELPERS};
use swc_core::ecma::transforms::base::{resolver, Assumptions};
use swc_core::ecma::transforms::compat::reserved_words;
use swc_core::ecma::transforms::optimization::simplifier;
use swc_core::ecma::transforms::optimization::simplify::{dce, Config as SimpilifyConfig};
use swc_core::ecma::transforms::proposal::decorators;
use swc_core::ecma::visit::{Fold, VisitMut};
use swc_error_reporters::handler::try_with_handler;

use crate::ast::css_ast::CssAst;
use crate::ast::file::File;
Expand All @@ -23,6 +26,7 @@ use crate::compiler::Context;
use crate::config::Mode;
use crate::features;
use crate::module::ModuleAst;
use crate::plugin::PluginTransformJsParam;
use crate::plugins::context_module::ContextModuleVisitor;
use crate::visitors::css_assets::CSSAssets;
use crate::visitors::css_flexbugs::CSSFlexbugs;
Expand Down Expand Up @@ -107,7 +111,7 @@ impl Transform {
&& is_browser;
if is_jsx {
visitors.push(react(
cm,
cm.clone(),
context.clone(),
use_refresh,
&top_level_mark,
Expand Down Expand Up @@ -169,34 +173,6 @@ impl Transform {
let comments = origin_comments.get_swc_comments().clone();
let assumptions = context.assumptions_for(file);

folders.push(Box::new(swc_preset_env::preset_env(
unresolved_mark,
Some(comments),
swc_preset_env::Config {
mode: Some(swc_preset_env::Mode::Entry),
targets: Some(swc_preset_env_targets_from_map(
context.config.targets.clone(),
)),
..Default::default()
},
assumptions,
&mut FeatureFlag::default(),
)));
folders.push(Box::new(reserved_words::reserved_words()));
folders.push(Box::new(paren_remover(Default::default())));
// simplify, but keep top level dead code
// e.g. import x from 'foo'; but x is not used
// this must be kept for tree shaking to work
folders.push(Box::new(simplifier(
unresolved_mark,
SimpilifyConfig {
dce: dce::Config {
top_level: false,
..Default::default()
},
..Default::default()
},
)));
// NOTICE: remove optimize_package_imports temporarily
// folders.push(Box::new(Optional {
// enabled: should_optimize(file.path.to_str().unwrap(), context.clone()),
Expand All @@ -206,7 +182,59 @@ impl Transform {
// ),
// }));

ast.transform(&mut visitors, &mut folders, file, true, context.clone())?;
ast.transform(&mut visitors, &mut folders, false, context.clone())?;

// run transform js in plugins
try_with_handler(cm.clone(), Default::default(), |handler| {
HELPERS.set(&Helpers::new(true), || {
HANDLER.set(handler, || {
// transform with plugin
context.plugin_driver.transform_js(
&PluginTransformJsParam {
handler,
path: file.path.to_str().unwrap(),
top_level_mark,
unresolved_mark,
},
&mut ast.ast,
&context,
)
})
})
})?;
Copy link
Contributor

Choose a reason for hiding this comment

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

集成插件转换

通过 try_with_handler 设置错误处理上下文,并调用 plugin_driver.transform_js 方法进行插件转换。这种集成增强了转换过程的灵活性和可扩展性。


// preset_env should go last
let mut preset_folders: Vec<Box<dyn Fold>> = vec![
Box::new(swc_preset_env::preset_env(
unresolved_mark,
Some(comments),
swc_preset_env::Config {
mode: Some(swc_preset_env::Mode::Entry),
targets: Some(swc_preset_env_targets_from_map(
context.config.targets.clone(),
)),
..Default::default()
},
assumptions,
&mut FeatureFlag::default(),
)),
Box::new(reserved_words::reserved_words()),
Box::new(paren_remover(Default::default())),
// simplify, but keep top level dead code
// e.g. import x from 'foo'; but x is not used
// this must be kept for tree shaking to work
Box::new(simplifier(
unresolved_mark,
SimpilifyConfig {
dce: dce::Config {
top_level: false,
..Default::default()
},
..Default::default()
},
)),
];
ast.transform(&mut vec![], &mut preset_folders, true, context.clone())?;

Ok(())
})
Expand Down
4 changes: 2 additions & 2 deletions crates/mako/src/plugins/emotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use anyhow::Ok;
use swc_core::common::comments::NoopComments;
use swc_core::common::sync::Lrc;
use swc_core::common::util::take::Take;
use swc_core::common::SourceMap;
use swc_core::ecma::ast::Module;
use swc_core::ecma::visit::{Fold, VisitMut, VisitMutWith};
Expand Down Expand Up @@ -77,8 +78,7 @@
self.cm.clone(),
NoopComments,
);
module.body = folder.fold_module(module.clone()).body;

module.visit_mut_children_with(self);
*module = folder.fold_module(module.take());

Check warning on line 82 in crates/mako/src/plugins/emotion.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/plugins/emotion.rs#L82

Added line #L82 was not covered by tests
stormslowly marked this conversation as resolved.
Show resolved Hide resolved
}
}
4 changes: 1 addition & 3 deletions crates/mako/src/visitors/fix_helper_inject_position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ mod tests {
use swc_core::ecma::visit::{Fold, VisitMut, VisitMutWith};

use super::FixHelperInjectPosition;
use crate::ast::file::File;
use crate::ast::tests::TestUtils;
use crate::build::targets::swc_preset_env_targets_from_map;

Expand Down Expand Up @@ -190,8 +189,7 @@ export { foo };
)));
let mut visitors: Vec<Box<dyn VisitMut>> = vec![];
let context = test_utils.context.clone();
let file = File::new("test.ts".to_string(), context.clone());
ast.transform(&mut visitors, &mut folders, &file, false, context)
ast.transform(&mut visitors, &mut folders, false, context)
.unwrap();
});
test_utils.js_ast_to_code()
Expand Down
4 changes: 4 additions & 0 deletions e2e/fixtures/config.emotion/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@ const test = async () => {
}
};
module.exports = test;

if (!module.parent) {
test();
}
6 changes: 5 additions & 1 deletion e2e/fixtures/config.emotion/mako.config.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
{
"emotion": true
"emotion": true,
"targets": {
"chrome": 40
},
"progress": false
}
Loading