From 47cd07bb75cb8c50fda3680d0f13a796b0e5b687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Thu, 16 Dec 2021 20:23:31 +0900 Subject: [PATCH] fix(next-swc/styled-jsx): Fix interpolation in media query (#32490) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/next-swc/crates/core/src/lib.rs | 10 ++-- .../crates/core/src/styled_jsx/mod.rs | 35 ++++++++++++-- .../core/src/styled_jsx/transform_css.rs | 35 +++++++++++--- .../next-swc/crates/core/tests/fixture.rs | 26 ++++------- .../input.js | 30 ++++++++++++ .../output.js | 46 +++++++++++++++++++ packages/next-swc/crates/core/tests/full.rs | 2 +- .../next-swc/crates/napi/src/transform.rs | 4 +- packages/next-swc/crates/wasm/src/lib.rs | 2 +- 9 files changed, 156 insertions(+), 34 deletions(-) create mode 100644 packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/input.js create mode 100644 packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/output.js diff --git a/packages/next-swc/crates/core/src/lib.rs b/packages/next-swc/crates/core/src/lib.rs index e2ba8d623dcdae2..1a36b3b2b5693e0 100644 --- a/packages/next-swc/crates/core/src/lib.rs +++ b/packages/next-swc/crates/core/src/lib.rs @@ -36,8 +36,8 @@ use std::cell::RefCell; use std::rc::Rc; use std::{path::PathBuf, sync::Arc}; use swc::config::ModuleConfig; -use swc_common::SourceFile; use swc_common::{self, chain, pass::Optional}; +use swc_common::{SourceFile, SourceMap}; use swc_ecmascript::ast::EsVersion; use swc_ecmascript::transforms::pass::noop; use swc_ecmascript::{ @@ -95,10 +95,14 @@ pub struct TransformOptions { pub shake_exports: Option, } -pub fn custom_before_pass(file: Arc, opts: &TransformOptions) -> impl Fold { +pub fn custom_before_pass( + cm: Arc, + file: Arc, + opts: &TransformOptions, +) -> impl Fold { chain!( disallow_re_export_all_in_page::disallow_re_export_all_in_page(opts.is_page_file), - styled_jsx::styled_jsx(), + styled_jsx::styled_jsx(cm.clone()), hook_optimizer::hook_optimizer(), match &opts.styled_components { Some(config) => { diff --git a/packages/next-swc/crates/core/src/styled_jsx/mod.rs b/packages/next-swc/crates/core/src/styled_jsx/mod.rs index 80973b2b599e110..058b7da77652793 100644 --- a/packages/next-swc/crates/core/src/styled_jsx/mod.rs +++ b/packages/next-swc/crates/core/src/styled_jsx/mod.rs @@ -2,6 +2,8 @@ use easy_error::{bail, Error}; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; use std::mem::take; +use std::sync::Arc; +use swc_common::SourceMap; use swc_common::{collections::AHashSet, Span, DUMMY_SP}; use swc_ecmascript::ast::*; use swc_ecmascript::minifier::{ @@ -23,12 +25,30 @@ use utils::*; mod transform_css; mod utils; -pub fn styled_jsx() -> impl Fold { - StyledJSXTransformer::default() +pub fn styled_jsx(cm: Arc) -> impl Fold { + StyledJSXTransformer { + cm, + styles: Default::default(), + static_class_name: Default::default(), + class_name: Default::default(), + file_has_styled_jsx: Default::default(), + has_styled_jsx: Default::default(), + bindings: Default::default(), + nearest_scope_bindings: Default::default(), + func_scope_level: Default::default(), + style_import_name: Default::default(), + external_bindings: Default::default(), + file_has_css_resolve: Default::default(), + external_hash: Default::default(), + add_hash: Default::default(), + add_default_decl: Default::default(), + in_function_params: Default::default(), + evaluator: Default::default(), + } } -#[derive(Default)] struct StyledJSXTransformer { + cm: Arc, styles: Vec, static_class_name: Option, class_name: Option, @@ -489,7 +509,12 @@ impl StyledJSXTransformer { match &style_info { JSXStyle::Local(style_info) => { - let css = transform_css(&style_info, is_global, &self.static_class_name)?; + let css = transform_css( + self.cm.clone(), + &style_info, + is_global, + &self.static_class_name, + )?; Ok(make_local_styled_jsx_el( &style_info, css, @@ -537,7 +562,7 @@ impl StyledJSXTransformer { } else { bail!("This shouldn't happen, we already know that this is a template literal"); }; - let css = transform_css(&style, tag == "global", &static_class_name)?; + let css = transform_css(self.cm.clone(), &style, tag == "global", &static_class_name)?; if tag == "resolve" { self.file_has_css_resolve = true; return Ok(Expr::Object(ObjectLit { diff --git a/packages/next-swc/crates/core/src/styled_jsx/transform_css.rs b/packages/next-swc/crates/core/src/styled_jsx/transform_css.rs index a902cf825f68d7f..dcf00d73bf56b7e 100644 --- a/packages/next-swc/crates/core/src/styled_jsx/transform_css.rs +++ b/packages/next-swc/crates/core/src/styled_jsx/transform_css.rs @@ -1,6 +1,8 @@ use easy_error::{bail, Error}; use std::panic; +use std::sync::Arc; use swc_common::util::take::Take; +use swc_common::SourceMap; use swc_common::{source_map::Pos, BytePos, Span, SyntaxContext, DUMMY_SP}; use swc_css::ast::*; use swc_css::codegen::{ @@ -19,6 +21,7 @@ use tracing::{debug, trace}; use super::{hash_string, string_literal_expr, LocalStyle}; pub fn transform_css( + cm: Arc, style_info: &LocalStyle, is_global: bool, class_name: &Option, @@ -59,7 +62,7 @@ pub fn transform_css( }; // ? Do we need to support optionally prefixing? ss.visit_mut_with(&mut prefixer()); - ss.visit_mut_with(&mut CssPlaceholderFixer); + ss.visit_mut_with(&mut CssPlaceholderFixer { cm }); ss.visit_mut_with(&mut Namespacer { class_name: match class_name { Some(s) => s.clone(), @@ -131,7 +134,9 @@ fn read_number(s: &str) -> (usize, usize) { /// This fixes invalid css which is created from interpolated expressions. /// /// `__styled-jsx-placeholder-` is handled at here. -struct CssPlaceholderFixer; +struct CssPlaceholderFixer { + cm: Arc, +} impl VisitMut for CssPlaceholderFixer { fn visit_mut_media_query(&mut self, q: &mut MediaQuery) { @@ -139,10 +144,28 @@ impl VisitMut for CssPlaceholderFixer { match q { MediaQuery::Ident(q) => { - if q.raw.starts_with("__styled-jsx-placeholder-") { - // TODO(kdy1): Remove this once we have CST for media query. - // We need good error recovery for media queries to handle this. - q.raw = format!("({})", &q.value).into(); + if !q.raw.starts_with("__styled-jsx-placeholder-") { + return; + } + // We need to support both of @media ($breakPoint) {} and @media $queryString {} + // This is complex because @media (__styled-jsx-placeholder-0__) {} is valid + // while @media __styled-jsx-placeholder-0__ {} is not + // + // So we check original source code to determine if we should inject + // parenthesis. + + // TODO(kdy1): Avoid allocation. + // To remove allocation, we should patch swc_common to provide a way to get + // source code without allocation. + // + // + // We need + // + // fn with_source_code (self: &mut Self, f: impl FnOnce(&str) -> Ret) -> _ {} + if let Ok(source) = self.cm.span_to_snippet(q.span) { + if source.starts_with('(') { + q.raw = format!("({})", &q.value).into(); + } } } _ => {} diff --git a/packages/next-swc/crates/core/tests/fixture.rs b/packages/next-swc/crates/core/tests/fixture.rs index 863cfb5e5e01845..e43e4a48a9cf822 100644 --- a/packages/next-swc/crates/core/tests/fixture.rs +++ b/packages/next-swc/crates/core/tests/fixture.rs @@ -14,7 +14,6 @@ use swc_ecma_transforms_testing::{test, test_fixture}; use swc_ecmascript::{ parser::{EsConfig, Syntax}, transforms::{react::jsx, resolver}, - visit::as_folder, }; use testing::fixture; @@ -99,26 +98,14 @@ fn styled_jsx_fixture(input: PathBuf) { let output = input.parent().unwrap().join("output.js"); test_fixture( syntax(), - &|_tr| chain!(resolver(), styled_jsx()), + &|t| chain!(resolver(), styled_jsx(t.cm.clone())), &input, &output, ); -} -pub struct DropSpan; -impl swc_ecmascript::visit::VisitMut for DropSpan { - fn visit_mut_span(&mut self, span: &mut Span) { - *span = DUMMY_SP - } -} - -/// Hash of styled-jsx should not depend on the span of expressions. -#[fixture("tests/fixture/styled-jsx/**/input.js")] -fn styled_jsx_span_should_not_affect_hash(input: PathBuf) { - let output = input.parent().unwrap().join("output.js"); test_fixture( syntax(), - &|_tr| { + &|t| { // `resolver` uses `Mark` which is stored in a thread-local storage (namely // swc_common::GLOBALS), and this loop will make `Mark` to be different from the // invocation above. @@ -128,13 +115,20 @@ fn styled_jsx_span_should_not_affect_hash(input: PathBuf) { let _mark = Mark::fresh(Mark::root()); } - chain!(as_folder(DropSpan), resolver(), styled_jsx()) + chain!(resolver(), styled_jsx(t.cm.clone())) }, &input, &output, ); } +pub struct DropSpan; +impl swc_ecmascript::visit::VisitMut for DropSpan { + fn visit_mut_span(&mut self, span: &mut Span) { + *span = DUMMY_SP + } +} + #[fixture("tests/fixture/page-config/**/input.js")] fn page_config_fixture(input: PathBuf) { let output = input.parent().unwrap().join("output.js"); diff --git a/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/input.js b/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/input.js new file mode 100644 index 000000000000000..39b930b9c7c2086 --- /dev/null +++ b/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/input.js @@ -0,0 +1,30 @@ + + + +export default class { + render() { + return ( +
+

test

+ +
+ ) + } + } + \ No newline at end of file diff --git a/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/output.js b/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/output.js new file mode 100644 index 000000000000000..d44f9c1b2cfb1e8 --- /dev/null +++ b/packages/next-swc/crates/core/tests/fixture/styled-jsx/issue-31562-interpolation-in-mdea/output.js @@ -0,0 +1,46 @@ +import _JSXStyle from "styled-jsx/style"; +export default class { + render() { + return
+ +

test

+ + <_JSXStyle id={"a876575397b32eda"} dynamic={[ + Typography.base.size.default, + Typography.base.lineHeight, + Target.mediumPlus, + Typography.base.size.mediumPlus, + Target.largePlus, + Typography.base.size.largePlus + ]}>{`html{font-size:${Typography.base.size.default}; +line-height:${Typography.base.lineHeight}} +@media ${Target.mediumPlus} {html{font-size:${Typography.base.size.mediumPlus}}} +@media ${Target.largePlus} {html{font-size:${Typography.base.size.largePlus}}}`} + +
; + } +}; diff --git a/packages/next-swc/crates/core/tests/full.rs b/packages/next-swc/crates/core/tests/full.rs index 773aad547d8d39d..834b14748d920a1 100644 --- a/packages/next-swc/crates/core/tests/full.rs +++ b/packages/next-swc/crates/core/tests/full.rs @@ -69,7 +69,7 @@ fn test(input: &Path, minify: bool) { None, &handler, &options.swc, - |_| custom_before_pass(fm.clone(), &options), + |_| custom_before_pass(cm.clone(), fm.clone(), &options), |_| noop(), ) { Ok(v) => { diff --git a/packages/next-swc/crates/napi/src/transform.rs b/packages/next-swc/crates/napi/src/transform.rs index 93c290a5cfcc628..4992542638aa013 100644 --- a/packages/next-swc/crates/napi/src/transform.rs +++ b/packages/next-swc/crates/napi/src/transform.rs @@ -30,9 +30,9 @@ use crate::{ complete_output, get_compiler, util::{deserialize_json, CtxtExt, MapErr}, }; -use next_swc::{custom_before_pass, TransformOptions}; use anyhow::{anyhow, bail, Context as _, Error}; use napi::{CallContext, Env, JsBoolean, JsBuffer, JsObject, JsString, JsUnknown, Status, Task}; +use next_swc::{custom_before_pass, TransformOptions}; use std::fs::read_to_string; use std::{ convert::TryFrom, @@ -94,7 +94,7 @@ impl Task for TransformTask { }; let options = options.patch(&fm); - let before_pass = custom_before_pass(fm.clone(), &options); + let before_pass = custom_before_pass(self.c.cm.clone(), fm.clone(), &options); self.c.process_js_with_custom_pass( fm.clone(), None, diff --git a/packages/next-swc/crates/wasm/src/lib.rs b/packages/next-swc/crates/wasm/src/lib.rs index c1359a27a9a111d..1273e36577f1d70 100644 --- a/packages/next-swc/crates/wasm/src/lib.rs +++ b/packages/next-swc/crates/wasm/src/lib.rs @@ -47,7 +47,7 @@ pub fn transform_sync(s: &str, opts: JsValue) -> Result { }, s.into(), ); - let before_pass = custom_before_pass(fm.clone(), &opts); + let before_pass = custom_before_pass(c.cm.clone(), fm.clone(), &opts); let out = c .process_js_with_custom_pass(fm, None, &handler, &opts.swc, |_| before_pass, |_| noop()) .context("failed to process js file")?;