From 53bdbc207d2b9175c76fd4ef8b18374701e542e2 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 2 Oct 2026 09:19:34 +0900 Subject: [PATCH] fix(extractor): map literal-key theme reads to variables and reject unmapped ones Refs #689 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .changepacks/changepack_log_theme_reads.json | 7 ++ bindings/devup-ui-wasm/src/lib.rs | 8 +- libs/css/src/theme_tokens.rs | 2 +- libs/extractor/src/css_utils.rs | 85 ++++++++++++++++++- .../extractor/extract_style_from_styled.rs | 7 ++ libs/extractor/src/lib.rs | 54 ++++++++++-- ...nents_theme_resolves_to_css_variables.snap | 28 +++--- libs/extractor/src/tailwind.rs | 4 +- 8 files changed, 162 insertions(+), 33 deletions(-) create mode 100644 .changepacks/changepack_log_theme_reads.json diff --git a/.changepacks/changepack_log_theme_reads.json b/.changepacks/changepack_log_theme_reads.json new file mode 100644 index 000000000..27dd8317c --- /dev/null +++ b/.changepacks/changepack_log_theme_reads.json @@ -0,0 +1,7 @@ +{ + "changes": { + "bindings/devup-ui-wasm/package.json": "Patch" + }, + "note": "A styled-components or Emotion style function reading the theme by literal keys, such as `p => p.theme.space[2]` or `({ theme }) => theme.colors['brand']`, now reads the CSS variable ThemeProvider declares (`var(--space-2)`). A theme read the build cannot turn into a CSS variable, such as `p => p.theme`, `p => p.theme.brand()` or `theme.space[i]`, is a build error naming the file, line, column and code, where it used to read a theme object that does not exist at runtime and give undefined or throw", + "date": "2026-10-01T00:00:00.000Z" +} diff --git a/bindings/devup-ui-wasm/src/lib.rs b/bindings/devup-ui-wasm/src/lib.rs index 4560410f3..0a2347ac2 100644 --- a/bindings/devup-ui-wasm/src/lib.rs +++ b/bindings/devup-ui-wasm/src/lib.rs @@ -1551,7 +1551,7 @@ mod tests { ); // Test getters - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert_eq!(output.css_file(), Some("devup-ui-0.css".to_string())); assert_eq!(output.map(), Some("//# sourceMappingURL=test".to_string())); assert!(output.css().is_some()); @@ -1951,7 +1951,7 @@ mod tests { assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert!(output.map().is_some()); } @@ -1975,7 +1975,7 @@ mod tests { assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert!(output.map().is_none()); } @@ -2000,7 +2000,7 @@ mod tests { assert!(result.is_err()); if let Err(error) = result { - assert!(!error.is_empty()); + assert_ne!(error, ""); } } diff --git a/libs/css/src/theme_tokens.rs b/libs/css/src/theme_tokens.rs index d8d8fb021..3b2346aed 100644 --- a/libs/css/src/theme_tokens.rs +++ b/libs/css/src/theme_tokens.rs @@ -119,7 +119,7 @@ mod tests { set_typography_keys(vec!["body".to_string(), "title".to_string()]); assert_eq!(get_typography_keys(), vec!["body", "title"]); set_typography_keys(vec![]); - assert!(get_typography_keys().is_empty()); + assert_eq!(get_typography_keys(), Vec::::new()); } #[test] diff --git a/libs/extractor/src/css_utils.rs b/libs/extractor/src/css_utils.rs index f0d67200b..bc6e43c07 100644 --- a/libs/extractor/src/css_utils.rs +++ b/libs/extractor/src/css_utils.rs @@ -69,12 +69,17 @@ pub(crate) fn theme_var_reference(expr: &Expression<'_>) -> Option { _ => return None, }; - let mut path = Vec::new(); + let mut path: Vec> = Vec::new(); let mut cursor = arrow.body.as_expression()?; loop { match cursor { Expression::StaticMemberExpression(member) => { - path.push(member.property.name.as_str()); + path.push(Cow::Borrowed(member.property.name.as_str())); + cursor = &member.object; + } + // `theme.space[2]` and `theme.colors['brand']` read a key the build knows + Expression::ComputedMemberExpression(member) => { + path.push(get_string_by_literal_expression(&member.expression)?); cursor = &member.object; } Expression::Identifier(ident) => { @@ -100,6 +105,82 @@ pub(crate) fn theme_var_reference(expr: &Expression<'_>) -> Option { Some(format!("var(--{})", path.join("-"))) } +/// Whether the style function `expr` reads the theme in a way +/// [`theme_var_reference`] cannot turn into a CSS variable, such as +/// `p => p.theme.brand()` or `({ theme }) => theme.space[i]`: no theme object +/// exists at runtime, so it would read `undefined` or throw +pub(crate) fn reads_unmapped_theme(expr: &Expression<'_>) -> bool { + use oxc_ast_visit::{Visit, walk}; + struct ThemeReads<'n> { + props: Option<&'n str>, + found: bool, + } + impl<'a> Visit<'a> for ThemeReads<'_> { + fn visit_static_member_expression( + &mut self, + it: &oxc_ast::ast::StaticMemberExpression<'a>, + ) { + if it.property.name == "theme" + && matches!(&it.object, Expression::Identifier(object) if Some(object.name.as_str()) == self.props) + { + self.found = true; + } + walk::walk_static_member_expression(self, it); + } + fn visit_identifier_reference(&mut self, it: &oxc_ast::ast::IdentifierReference<'a>) { + if self.props.is_none() && it.name == "theme" { + self.found = true; + } + } + } + let Expression::ArrowFunctionExpression(arrow) = expr else { + return false; + }; + let [param] = arrow.params.items.as_slice() else { + return false; + }; + let props = match ¶m.pattern { + BindingPattern::BindingIdentifier(ident) => Some(ident.name.as_str()), + BindingPattern::ObjectPattern(pattern) + if pattern + .properties + .iter() + .any(|p| get_str_by_property_key(&p.key).is_some_and(|key| key == "theme")) => + { + None + } + _ => return false, + }; + if theme_var_reference(expr).is_some() { + return false; + } + let mut reads = ThemeReads { + props, + found: false, + }; + match &arrow.body { + oxc_ast::ast::ArrowFunctionBody::FunctionBody(body) => reads.visit_function_body(body), + body => { + if let Some(expression) = body.as_expression() { + reads.visit_expression(expression); + } + } + } + reads.found +} + +/// The build error for a theme read [`reads_unmapped_theme`] finds +pub(crate) fn unmapped_theme_error(api: &str, expr: &Expression<'_>) -> (u32, String) { + ( + expr.span().start, + crate::utils::build_time_error( + api, + &readable_code(expr), + "a theme read must be a path of names or literal keys, such as `p => p.theme.colors.brand` or `({ theme }) => theme.space[2]`, which reads the CSS variable `ThemeProvider` declares; compute other values outside the style", + ), + ) +} + enum ThemeRoot<'a> { Props(&'a str), Theme, diff --git a/libs/extractor/src/extractor/extract_style_from_styled.rs b/libs/extractor/src/extractor/extract_style_from_styled.rs index 425ed0245..6ed488326 100644 --- a/libs/extractor/src/extractor/extract_style_from_styled.rs +++ b/libs/extractor/src/extractor/extract_style_from_styled.rs @@ -225,6 +225,13 @@ pub fn extract_style_from_styled<'a>( let expression = &tag.quasi.expressions[index]; errors.push((expression.span().start, unplaced_error(expression))); } + errors.extend( + tag.quasi + .expressions + .iter() + .filter(|expression| crate::css_utils::reads_unmapped_theme(expression)) + .map(|expression| crate::css_utils::unmapped_theme_error("styled", expression)), + ); let mut props_styles: Vec> = styles .into_iter() .map(|ex| ExtractStyleProp::Static(ex.into())) diff --git a/libs/extractor/src/lib.rs b/libs/extractor/src/lib.rs index 68981f36c..4040f6604 100644 --- a/libs/extractor/src/lib.rs +++ b/libs/extractor/src/lib.rs @@ -836,8 +836,8 @@ mod tests { alternate: None, }; - assert!(empty.extract().is_empty()); - assert!(empty.into_extract().is_empty()); + assert_eq!(empty.extract(), vec![]); + assert_eq!(empty.into_extract(), vec![]); } #[test] @@ -13598,7 +13598,7 @@ globalCss({ ); assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code.is_empty()); + assert_ne!(output.code, ""); } #[test] @@ -18778,7 +18778,7 @@ export const k = styled('div')({ color: SIZE });", &memory_resolver(CONSTANT_MODULES), ) .unwrap(); - assert!(without_imports.dependencies.is_empty()); + assert_eq!(without_imports.dependencies.len(), 0); let without_constants = extract_with_modules( "/src/Handler.tsx", "import { Box } from '@devup-ui/react';\nimport { handler } from './handler';\nexport const a = ;", @@ -20544,11 +20544,10 @@ const Nested = styled.span`color: ${p => p.theme.colors.brand};`; const Destructured = styled.p`color: ${({ theme }) => theme.colors.accent};`; const Surrounded = styled.b`border: 1px solid ${p => p.theme.line};`; const NotTheme = styled.i`color: ${p => p.color};`; -const BareTheme = styled.u`color: ${p => p.theme};`; const OtherRoot = styled.s`color: ${p => q.theme.brand};`; const ArrayParam = styled.q`color: ${([p]) => p.theme.brand};`; const NoParam = styled.em`color: ${() => 'red'};`; -const CallBody = styled.strong`color: ${p => p.theme.brand()};`;", +const Indexed = styled.u`margin: ${p => p.theme.space[2]} ${({ theme }) => theme.colors['brand']};`;", ExtractOption { package: "@devup-ui/react".to_string(), css_dir: "@devup-ui/react".to_string(), @@ -20564,6 +20563,49 @@ const CallBody = styled.strong`color: ${p => p.theme.brand()};`;", )); } + // A theme read the build cannot turn into a CSS variable has no theme + // object to read at runtime + #[test] + #[serial] + fn test_styled_components_unmapped_theme_reads_are_errors() { + reset_class_map(); + reset_file_map(); + let code = match extract( + "test.tsx", + "import {styled} from '@devup-ui/core' +const A = styled.u`color: ${p => p.theme};` +const B = styled.b`color: ${p => p.theme.brand()};` +const C = styled.i`color: ${({ theme }) => theme.space[i]};` +const D = styled.s`color: ${({ theme }) => { return theme.a + 1 }};` +const E = styled.em`color: ${p => p.color};` +const F = styled.q`color: ${({ tone }) => tone};` +const G = styled.p`color: ${([p]) => p};` +const H = styled.a`color: ${(a, b) => a};` +const I = styled.dd`color: ${function (p) { return p.theme }};`", + ExtractOption { + package: "@devup-ui/core".to_string(), + css_dir: "@devup-ui/core".to_string(), + single_css: true, + import_main_css: false, + import_aliases: HashMap::new(), + }, + ) { + Ok(output) => output.code, + Err(error) => error.to_string(), + }; + for expected in [ + "test.tsx:2:29: `styled()` cannot use `(p) => p.theme` at build time: a theme read must be a path of names or literal keys", + "`(p) => p.theme.brand()`", + "`({ theme }) => theme.space[i]`", + "theme.a + 1", + ] { + assert!(code.contains(expected), "{expected}\n{code}"); + } + for unexpected in ["p.color", "tone", "([p])", "(a, b)", "function (p)"] { + assert!(!code.contains(unexpected), "{unexpected}\n{code}"); + } + } + #[test] #[serial] fn test_namespace_import_of_a_default_only_package_redirects() { diff --git a/libs/extractor/src/snapshots/extractor__tests__styled_components_theme_resolves_to_css_variables.snap b/libs/extractor/src/snapshots/extractor__tests__styled_components_theme_resolves_to_css_variables.snap index 777369542..25e59c338 100644 --- a/libs/extractor/src/snapshots/extractor__tests__styled_components_theme_resolves_to_css_variables.snap +++ b/libs/extractor/src/snapshots/extractor__tests__styled_components_theme_resolves_to_css_variables.snap @@ -1,6 +1,6 @@ --- source: libs/extractor/src/lib.rs -expression: "ToBTreeSet::from(extract(\"test.tsx\",\nr\"import styled from 'styled-components';\nconst Flat = styled.div`color: ${p => p.theme.brand};`;\nconst Nested = styled.span`color: ${p => p.theme.colors.brand};`;\nconst Destructured = styled.p`color: ${({ theme }) => theme.colors.accent};`;\nconst Surrounded = styled.b`border: 1px solid ${p => p.theme.line};`;\nconst NotTheme = styled.i`color: ${p => p.color};`;\nconst BareTheme = styled.u`color: ${p => p.theme};`;\nconst OtherRoot = styled.s`color: ${p => q.theme.brand};`;\nconst ArrayParam = styled.q`color: ${([p]) => p.theme.brand};`;\nconst NoParam = styled.em`color: ${() => 'red'};`;\nconst CallBody = styled.strong`color: ${p => p.theme.brand()};`;\",\nExtractOption\n{\n package: \"@devup-ui/react\".to_string(), css_dir:\n \"@devup-ui/react\".to_string(), single_css: true, import_main_css: false,\n import_aliases:\n HashMap::from([(\"styled-components\".to_string(),\n ImportAlias::DefaultToNamed(\"styled\".to_string()))])\n},).unwrap())" +expression: "ToBTreeSet::from(extract(\"test.tsx\",\nr\"import styled from 'styled-components';\nconst Flat = styled.div`color: ${p => p.theme.brand};`;\nconst Nested = styled.span`color: ${p => p.theme.colors.brand};`;\nconst Destructured = styled.p`color: ${({ theme }) => theme.colors.accent};`;\nconst Surrounded = styled.b`border: 1px solid ${p => p.theme.line};`;\nconst NotTheme = styled.i`color: ${p => p.color};`;\nconst OtherRoot = styled.s`color: ${p => q.theme.brand};`;\nconst ArrayParam = styled.q`color: ${([p]) => p.theme.brand};`;\nconst NoParam = styled.em`color: ${() => 'red'};`;\nconst Indexed = styled.u`margin: ${p => p.theme.space[2]} ${({ theme }) => theme.colors['brand']};`;\",\nExtractOption\n{\n package: \"@devup-ui/react\".to_string(), css_dir:\n \"@devup-ui/react\".to_string(), single_css: true, import_main_css: false,\n import_aliases:\n HashMap::from([(\"styled-components\".to_string(),\n ImportAlias::DefaultToNamed(\"styled\".to_string()))])\n},).unwrap())" --- ToBTreeSet { styles: { @@ -44,29 +44,21 @@ ToBTreeSet { layer: None, }, ), - Dynamic( - ExtractDynamicStyle { - property: "color", - level: 0, - identifier: "(()=>`red`)(rest)", - selector: None, - style_order: None, - }, - ), - Dynamic( - ExtractDynamicStyle { - property: "color", + Static( + ExtractStaticStyle { + property: "margin", + value: "var(--space-2) var(--colors-brand)", level: 0, - identifier: "(([p])=>p.theme.brand)(rest)", selector: None, style_order: None, + layer: None, }, ), Dynamic( ExtractDynamicStyle { property: "color", level: 0, - identifier: "(p=>p.color)(rest)", + identifier: "(()=>`red`)(rest)", selector: None, style_order: None, }, @@ -75,7 +67,7 @@ ToBTreeSet { ExtractDynamicStyle { property: "color", level: 0, - identifier: "(p=>p.theme)(rest)", + identifier: "(([p])=>p.theme.brand)(rest)", selector: None, style_order: None, }, @@ -84,7 +76,7 @@ ToBTreeSet { ExtractDynamicStyle { property: "color", level: 0, - identifier: "(p=>p.theme.brand())(rest)", + identifier: "(p=>p.color)(rest)", selector: None, style_order: None, }, @@ -99,5 +91,5 @@ ToBTreeSet { }, ), }, - code: "import \"@devup-ui/react/devup-ui.css\";\nconst Flat = ({ style, className, ...rest }) =>
;\nconst Nested = ({ style, className, ...rest }) => ;\nconst Destructured = ({ style, className, ...rest }) =>

;\nconst Surrounded = ({ style, className, ...rest }) => ;\nconst NotTheme = ({ style, className, ...rest }) => p.color)(rest) },\n\t...style\n}} />;\nconst BareTheme = ({ style, className, ...rest }) => p.theme)(rest) },\n\t...style\n}} />;\nconst OtherRoot = ({ style, className, ...rest }) => q.theme.brand)(rest) },\n\t...style\n}} />;\nconst ArrayParam = ({ style, className, ...rest }) => p.theme.brand)(rest) },\n\t...style\n}} />;\nconst NoParam = ({ style, className, ...rest }) => `red`)(rest) },\n\t...style\n}} />;\nconst CallBody = ({ style, className, ...rest }) => p.theme.brand())(rest) },\n\t...style\n}} />;\n", + code: "import \"@devup-ui/react/devup-ui.css\";\nconst Flat = ({ style, className, ...rest }) =>

;\nconst Nested = ({ style, className, ...rest }) => ;\nconst Destructured = ({ style, className, ...rest }) =>

;\nconst Surrounded = ({ style, className, ...rest }) => ;\nconst NotTheme = ({ style, className, ...rest }) => p.color)(rest) },\n\t...style\n}} />;\nconst OtherRoot = ({ style, className, ...rest }) => q.theme.brand)(rest) },\n\t...style\n}} />;\nconst ArrayParam = ({ style, className, ...rest }) => p.theme.brand)(rest) },\n\t...style\n}} />;\nconst NoParam = ({ style, className, ...rest }) => `red`)(rest) },\n\t...style\n}} />;\nconst Indexed = ({ style, className, ...rest }) => ;\n", } diff --git a/libs/extractor/src/tailwind.rs b/libs/extractor/src/tailwind.rs index 87e9f5178..70643aa7c 100644 --- a/libs/extractor/src/tailwind.rs +++ b/libs/extractor/src/tailwind.rs @@ -284,7 +284,7 @@ pub struct TailwindClass { /// non-overlapping) but mutates the existing buffer instead of allocating a new /// `String`. `needle` must be non-empty. fn remove_all_substr(haystack: &mut String, needle: &str) { - debug_assert!(!needle.is_empty()); + debug_assert_ne!(needle, ""); let mut search_from = 0; while let Some(rel) = haystack[search_from..].find(needle) { let at = search_from + rel; @@ -3934,7 +3934,7 @@ mod tests { #[test] fn test_empty_string() { let styles = parse_tailwind_to_styles(""); - assert!(styles.is_empty()); + assert_eq!(styles, vec![]); } #[test]