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(next-swc/styled-jsx): Fix interpolation in media query #32490

Merged
merged 7 commits into from Dec 16, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/next-swc/crates/core/src/lib.rs
Expand Up @@ -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::{
Expand Down Expand Up @@ -92,10 +92,14 @@ pub struct TransformOptions {
pub shake_exports: Option<shake_exports::Config>,
}

pub fn custom_before_pass(file: Arc<SourceFile>, opts: &TransformOptions) -> impl Fold {
pub fn custom_before_pass(
cm: Arc<SourceMap>,
file: Arc<SourceFile>,
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) => {
Expand Down
35 changes: 30 additions & 5 deletions packages/next-swc/crates/core/src/styled_jsx/mod.rs
Expand Up @@ -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::{
Expand All @@ -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<SourceMap>) -> 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<SourceMap>,
styles: Vec<JSXStyle>,
static_class_name: Option<String>,
class_name: Option<Expr>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 29 additions & 6 deletions 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::{
Expand All @@ -19,6 +21,7 @@ use tracing::{debug, trace};
use super::{hash_string, string_literal_expr, LocalStyle};

pub fn transform_css(
cm: Arc<SourceMap>,
style_info: &LocalStyle,
is_global: bool,
class_name: &Option<String>,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -131,18 +134,38 @@ 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<SourceMap>,
}

impl VisitMut for CssPlaceholderFixer {
fn visit_mut_media_query(&mut self, q: &mut MediaQuery) {
q.visit_mut_children_with(self);

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();
}
}
}
_ => {}
Expand Down
26 changes: 10 additions & 16 deletions packages/next-swc/crates/core/tests/fixture.rs
Expand Up @@ -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;

Expand Down Expand Up @@ -83,26 +82,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.
Expand All @@ -112,13 +99,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()))
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 removed DropSpan because styled-jsx depends on span.

},
&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");
Expand Down
@@ -0,0 +1,30 @@



export default class {
render() {
return (
<div>
<p>test</p>
<style jsx global>{`
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};
}
}
`}
</style>
</div>
)
}
}

@@ -0,0 +1,46 @@
import _JSXStyle from "styled-jsx/style";
export default class {
render() {
return <div className={_JSXStyle.dynamic([
[
"a876575397b32eda",
[
Typography.base.size.default,
Typography.base.lineHeight,
Target.mediumPlus,
Typography.base.size.mediumPlus,
Target.largePlus,
Typography.base.size.largePlus
]
]
])}>

<p className={_JSXStyle.dynamic([
[
"a876575397b32eda",
[
Typography.base.size.default,
Typography.base.lineHeight,
Target.mediumPlus,
Typography.base.size.mediumPlus,
Target.largePlus,
Typography.base.size.largePlus
]
]
])}>test</p>

<_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}}}`}</_JSXStyle>

</div>;
}
};
2 changes: 1 addition & 1 deletion packages/next-swc/crates/core/tests/full.rs
Expand Up @@ -68,7 +68,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) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/next-swc/crates/napi/src/transform.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/next-swc/crates/wasm/src/lib.rs
Expand Up @@ -47,7 +47,7 @@ pub fn transform_sync(s: &str, opts: JsValue) -> Result<JsValue, JsValue> {
},
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")?;
Expand Down