diff --git a/.changeset/fair-crews-lie.md b/.changeset/fair-crews-lie.md new file mode 100644 index 0000000000..755de48536 --- /dev/null +++ b/.changeset/fair-crews-lie.md @@ -0,0 +1,5 @@ +--- +"@lynx-js/react": patch +--- + +Improve `shake.removeCall` and `shake.removeCallParams` in the React transform so they also match aliased runtime imports such as `import { useEffect as myUseEffect } ...` and member calls such as `ReactLynxRuntime.useEffect(...)` from default or namespace runtime imports. diff --git a/packages/react/runtime/src/renderToOpcodes/index.ts b/packages/react/runtime/src/renderToOpcodes/index.ts index 475d595fd7..c6a9327135 100644 --- a/packages/react/runtime/src/renderToOpcodes/index.ts +++ b/packages/react/runtime/src/renderToOpcodes/index.ts @@ -26,6 +26,8 @@ import { RENDER, SKIP_EFFECTS, VNODE, + HOOK, + CHILD_DID_SUSPEND, } from './constants.js'; /** @typedef {import('preact').VNode} VNode */ @@ -87,7 +89,7 @@ export function renderToString(vnode: any, context: any, into: SnapshotInstance) // Installed as setState/forceUpdate for function components function markAsDirty() { - this.__d = true; + this[DIRTY] = true; } const EMPTY_OBJ = {}; @@ -165,9 +167,10 @@ function _renderToString( return; } + let vnodeType = typeof vnode; // Text VNodes: escape as HTML - if (typeof vnode !== 'object') { - if (typeof vnode === 'function') return; + if (vnodeType !== 'object') { + if (vnodeType === 'function') return; renderToTextNode(into, vnode + '', opcodes); return; } @@ -225,15 +228,15 @@ function _renderToString( component = vnode[COMPONENT]; } else { component = { - __v: vnode, + [VNODE]: vnode, props, context: cctx, // silently drop state updates setState: markAsDirty, forceUpdate: markAsDirty, - __d: true, + [DIRTY]: true, // hooks - __h: [], + [HOOK]: [], }; vnode[COMPONENT] = component; component.constructor = type; @@ -288,7 +291,7 @@ function _renderToString( ? lastChild.__nextSibling : into.__firstChild, ); - if (e && typeof e === 'object' && e.then && component && /* _childDidSuspend */ component.__c) { + if (e && typeof e === 'object' && e.then && component && /* _childDidSuspend */ component[CHILD_DID_SUSPEND]) { component.setState({ /* _suspended */ __a: true }); if (component[DIRTY]) { @@ -367,7 +370,8 @@ function _renderToString( } } - if (typeof children === 'string' || typeof children === 'number') { + let childrenType = typeof children; + if (childrenType === 'string' || childrenType === 'number') { // single text child renderToTextNode(vnode, children, opcodes); } else if (children != null && children !== false && children !== true) { diff --git a/packages/react/transform/crates/swc_plugin_shake/lib.rs b/packages/react/transform/crates/swc_plugin_shake/lib.rs index e91cc359a5..d57990693a 100644 --- a/packages/react/transform/crates/swc_plugin_shake/lib.rs +++ b/packages/react/transform/crates/swc_plugin_shake/lib.rs @@ -146,20 +146,18 @@ impl Default for ShakeVisitorConfig { pub struct ShakeVisitor { opts: ShakeVisitorConfig, - is_runtime_import: bool, current_method_name: Option, current_method_ref_names: Option>, - import_ids: Vec, + import_ids: HashMap, } impl ShakeVisitor { pub fn new(opts: ShakeVisitorConfig) -> Self { ShakeVisitor { opts, - is_runtime_import: false, current_method_name: None, current_method_ref_names: None, - import_ids: Vec::new(), + import_ids: HashMap::new(), } } @@ -169,10 +167,51 @@ impl ShakeVisitor { /// - `remove_call`, where the whole call expression is removed/replaced /// - `remove_call_params`, where only the call arguments are cleared fn should_remove_call(&self, n: &CallExpr, target_calls: &[String]) -> bool { - if let Some(fn_name) = n.callee.as_expr().and_then(|s| s.as_ident()) { - self.import_ids.contains(&fn_name.to_id()) && target_calls.contains(&fn_name.sym.to_string()) - } else { - false + let Some(callee) = n.callee.as_expr() else { + // Skip non-expression callees such as `super(...)` and `import(...)`. + return false; + }; + + // Case 1: direct calls like `useEffect(...)` or `myUseEffect(...)`. + if let Some(fn_name) = callee.as_ident() { + return self + .import_ids + .get(&fn_name.to_id()) + .is_some_and(|imported_name| { + target_calls.contains(imported_name) || target_calls.contains(&fn_name.sym.to_string()) + }); + } + + // Case 2: member calls like `ReactLynxRuntime.useEffect(...)`. + // We only support member access on top-level runtime imports collected in `import_ids`. + let Some(member) = callee.as_member() else { + return false; + }; + let Expr::Ident(object_ident) = &*member.obj else { + return false; + }; + let Some(imported_name) = self.import_ids.get(&object_ident.to_id()) else { + return false; + }; + + // Only `default import` and `namespace import` can legally produce runtime member calls. + if imported_name != "default" && imported_name != "*" { + return false; + } + + match &member.prop { + // Dot access: `ReactLynxRuntime.useEffect` + MemberProp::Ident(prop_ident) => target_calls.contains(&prop_ident.sym.to_string()), + // Computed string access: `ReactLynxRuntime["useEffect"]` + MemberProp::Computed(computed) => computed + .expr + .as_lit() + .and_then(|lit| match lit { + Lit::Str(str) => Some(str.value.to_string_lossy().into_owned()), + _ => None, + }) + .is_some_and(|prop_name| target_calls.contains(&prop_name)), + _ => false, } } } @@ -207,7 +246,8 @@ impl VisitMut for ShakeVisitor { } /** - * labeling import stmt + * Record runtime imports as `local_id -> imported_name` so aliased imports + * like `import { useEffect as myUseEffect }` still match `useEffect`. */ fn visit_mut_import_decl(&mut self, n: &mut ImportDecl) { let import_src = n.src.value.to_string_lossy(); @@ -217,17 +257,45 @@ impl VisitMut for ShakeVisitor { .iter() .any(|pkg| pkg == import_src.as_ref()) { - self.is_runtime_import = true; - } - n.visit_mut_children_with(self); - self.is_runtime_import = false; - } - /** - * labeling identifiers of the import by id - */ - fn visit_mut_ident(&mut self, n: &mut Ident) { - if self.is_runtime_import { - self.import_ids.push(n.to_id()) + for specifier in &n.specifiers { + match specifier { + ImportSpecifier::Named(named) => { + // Example: + // import { useEffect } from "@lynx-js/react-runtime"; + // import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; + // Result: + // import_ids[useEffect(local id)] = "useEffect" + // import_ids[myUseEffect(local id)] = "useEffect" + let imported_name = named + .imported + .as_ref() + .map(|imported| match imported { + ModuleExportName::Ident(ident) => ident.sym.to_string(), + ModuleExportName::Str(str) => str.value.to_string_lossy().into_owned(), + }) + .unwrap_or_else(|| named.local.sym.to_string()); + self.import_ids.insert(named.local.to_id(), imported_name); + } + ImportSpecifier::Default(default) => { + // Example: + // import ReactLynxRuntime from "@lynx-js/react-runtime"; + // Result: + // import_ids[ReactLynxRuntime(local id)] = "default" + self + .import_ids + .insert(default.local.to_id(), "default".to_string()); + } + ImportSpecifier::Namespace(namespace) => { + // Example: + // import * as ReactLynxRuntime from "@lynx-js/react-runtime"; + // Result: + // import_ids[ReactLynxRuntime(local id)] = "*" + self + .import_ids + .insert(namespace.local.to_id(), "*".to_string()); + } + } + } } n.visit_mut_children_with(self); } @@ -618,6 +686,140 @@ mod tests { "# ); + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: vec!["useEffect".to_string()], + remove_call_params: Vec::new(), + ..Default::default() + })), + should_remove_aliased_use_effect_call, + r#" + import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; + export function A () { + myUseEffect(()=>{ + console.log("keep aliased useEffect") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: vec!["myUseEffect".to_string()], + remove_call_params: Vec::new(), + ..Default::default() + })), + should_remove_aliased_use_effect_call_by_local_name, + r#" + import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; + export function A () { + myUseEffect(()=>{ + console.log("remove aliased useEffect by local name") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: vec!["useEffect".to_string()], + remove_call_params: Vec::new(), + ..Default::default() + })), + should_remove_default_and_namespace_runtime_import_calls, + r#" + import ReactLynxRuntime from "@lynx-js/react-runtime"; + import * as ReactLynxRuntimeNS from "@lynx-js/react-runtime"; + export function A () { + ReactLynxRuntime['useEffect'](()=>{ + console.log("remove default import member call") + }) + ReactLynxRuntimeNS.useEffect(()=>{ + console.log("remove namespace import member call") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: Vec::new(), + remove_call_params: vec!["useEffect".to_string()], + ..Default::default() + })), + should_remove_default_and_namespace_runtime_import_params, + r#" + import ReactLynxRuntime from "@lynx-js/react-runtime"; + import * as ReactLynxRuntimeNS from "@lynx-js/react-runtime"; + export function A () { + ReactLynxRuntime.useEffect(()=>{ + console.log("remove default import member call") + }) + ReactLynxRuntimeNS.useEffect(()=>{ + console.log("remove namespace import member call") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: Vec::new(), + remove_call_params: vec!["myUseEffect".to_string()], + ..Default::default() + })), + should_remove_aliased_use_effect_param_by_local_name, + r#" + import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; + export function A () { + myUseEffect(()=>{ + console.log("remove aliased useEffect by local name") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: Vec::new(), + remove_call_params: vec!["useEffect".to_string()], + ..Default::default() + })), + should_remove_aliased_use_effect_param, + r#" + import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; + export function A () { + myUseEffect(()=>{ + console.log("keep aliased useEffect") + }) + } + "# + ); + + test!( + Default::default(), + |_| visit_mut_pass(ShakeVisitor::new(ShakeVisitorConfig { + remove_call: vec!["useEffect".to_string()], + remove_call_params: Vec::new(), + ..Default::default() + })), + should_keep_use_effect_call_from_custom_runtime, + r#" + import { useEffect } from "@lynx-js/custom-react-runtime"; + export function A () { + useEffect(()=>{ + console.log("keep useEffect from custom runtime") + }) + } + "# + ); + test!( module, Syntax::Es(EsSyntax { diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_keep_use_effect_call_from_custom_runtime.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_keep_use_effect_call_from_custom_runtime.js new file mode 100644 index 0000000000..93bbd40da1 --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_keep_use_effect_call_from_custom_runtime.js @@ -0,0 +1,6 @@ +import { useEffect } from "@lynx-js/custom-react-runtime"; +export function A() { + useEffect(()=>{ + console.log("keep useEffect from custom runtime"); + }); +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call.js new file mode 100644 index 0000000000..747cfb744d --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call.js @@ -0,0 +1,4 @@ +import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; +export function A() { + ; +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call_by_local_name.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call_by_local_name.js new file mode 100644 index 0000000000..747cfb744d --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_call_by_local_name.js @@ -0,0 +1,4 @@ +import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; +export function A() { + ; +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param.js new file mode 100644 index 0000000000..6bebad8f6b --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param.js @@ -0,0 +1,4 @@ +import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; +export function A() { + myUseEffect(); +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param_by_local_name.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param_by_local_name.js new file mode 100644 index 0000000000..6bebad8f6b --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_aliased_use_effect_param_by_local_name.js @@ -0,0 +1,4 @@ +import { useEffect as myUseEffect } from "@lynx-js/react-runtime"; +export function A() { + myUseEffect(); +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_calls.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_calls.js new file mode 100644 index 0000000000..0d3ad020c4 --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_calls.js @@ -0,0 +1,6 @@ +import ReactLynxRuntime from "@lynx-js/react-runtime"; +import * as ReactLynxRuntimeNS from "@lynx-js/react-runtime"; +export function A() { + ; + ; +} diff --git a/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_params.js b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_params.js new file mode 100644 index 0000000000..e927d4e624 --- /dev/null +++ b/packages/react/transform/crates/swc_plugin_shake/tests/__swc_snapshots__/lib.rs/should_remove_default_and_namespace_runtime_import_params.js @@ -0,0 +1,6 @@ +import ReactLynxRuntime from "@lynx-js/react-runtime"; +import * as ReactLynxRuntimeNS from "@lynx-js/react-runtime"; +export function A() { + ReactLynxRuntime.useEffect(); + ReactLynxRuntimeNS.useEffect(); +} diff --git a/packages/webpack/react-webpack-plugin/src/loaders/options.ts b/packages/webpack/react-webpack-plugin/src/loaders/options.ts index 46ee1ba4c4..1aede58f07 100644 --- a/packages/webpack/react-webpack-plugin/src/loaders/options.ts +++ b/packages/webpack/react-webpack-plugin/src/loaders/options.ts @@ -238,6 +238,8 @@ export function getMainThreadTransformOptions( // so never pass true to shake to rust pkgName: [ 'react', + 'preact/hooks', + 'preact/compat', PUBLIC_RUNTIME_PKG, `${PUBLIC_RUNTIME_PKG}/legacy-react-runtime`, RUNTIME_PKG, diff --git a/packages/webpack/react-webpack-plugin/test/cases/compat/run-in-js/index.js b/packages/webpack/react-webpack-plugin/test/cases/compat/run-in-js/index.js index c02cb1a0e3..99942865c7 100644 --- a/packages/webpack/react-webpack-plugin/test/cases/compat/run-in-js/index.js +++ b/packages/webpack/react-webpack-plugin/test/cases/compat/run-in-js/index.js @@ -5,7 +5,7 @@ import ReactLynx, { __runInJS } from '@lynx-js/react-runtime'; it('should have __runInJS as named export', () => { expect(__runInJS).toStrictEqual(expect.any(Function)); if (__LEPUS__) { - // The argument will be removed + // The call expression will be removed expect(__runInJS(42)).toBeUndefined(); } else { expect(__runInJS(42)).toBe(42); @@ -14,5 +14,10 @@ it('should have __runInJS as named export', () => { it('should have __runInJS in the default export', () => { expect(ReactLynx).toHaveProperty('__runInJS', expect.any(Function)); - expect(ReactLynx.__runInJS(42)).toBe(42); + if (__LEPUS__) { + // The call expression will be removed + expect(ReactLynx.__runInJS(42)).toBeUndefined(); + } else { + expect(ReactLynx.__runInJS(42)).toBe(42); + } }); diff --git a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/lynx-js-react.js b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/lynx-js-react.js index 57fada3143..b3f2f6a35c 100644 --- a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/lynx-js-react.js +++ b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/lynx-js-react.js @@ -8,10 +8,10 @@ export default function App() { }, []); useMyEffect(() => { - // TODO: import alias is not removed + console.info('This should not exist in main-thread'); }); React.useEffect(() => { - // TODO: default import is not removed + console.info('This should not exist in main-thread'); }); } diff --git a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/react.js b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/react.js index 96d380cab1..c4359e9696 100644 --- a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/react.js +++ b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-effect/react.js @@ -8,10 +8,10 @@ export default function App() { }, []); useMyEffect(() => { - // TODO: import alias is not removed + console.info('This should not exist in main-thread'); }); React.useEffect(() => { - // TODO: default import is not removed + console.info('This should not exist in main-thread'); }); } diff --git a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/lynx-js-react.js b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/lynx-js-react.js index 1fad8b3531..55c03c1393 100644 --- a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/lynx-js-react.js +++ b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/lynx-js-react.js @@ -21,10 +21,10 @@ export default forwardRef(function App(_, ref) { }, []); useMyEffect(() => { - // TODO: import alias is not removed - }); + console.info('This should not exist in main-thread'); + }, []); React.useEffect(() => { - // TODO: default import is not removed - }); + console.info('This should not exist in main-thread'); + }, []); }); diff --git a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/react.js b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/react.js index f2709b6c9a..fb971740d3 100644 --- a/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/react.js +++ b/packages/webpack/react-webpack-plugin/test/cases/main-thread/use-imperative-handle/react.js @@ -21,10 +21,10 @@ export default forwardRef(function App(_, ref) { }, []); useMyEffect(() => { - // TODO: import alias is not removed + console.info('This should not exist in main-thread'); }); React.useEffect(() => { - // TODO: default import is not removed + console.info('This should not exist in main-thread'); }); });