Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
6 changes: 5 additions & 1 deletion crates/perry-codegen/src/lower_call/extern_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,11 @@ pub fn try_lower_extern_func_call(
// scope in #689 and continue to fall through to `js_jsx`; the runtime
// returns `undefined` for those unrecognised intrinsic sentinels until
// the rewriter is extended.
"jsx" | "jsxs" => {
"jsx" | "jsxs"
if !ctx.imported_vars.contains(name)
&& !ctx.import_function_prefixes.contains_key(name)
&& !ctx.import_function_v8_specifiers.contains_key(name) =>
{
if let Some(call) = try_rewrite_perry_tui_jsx_intrinsic(ctx, name == "jsxs", args)? {
return Ok(Some(call));
}
Expand Down
7 changes: 7 additions & 0 deletions crates/perry-hir/src/dynamic_import/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,13 @@ fn visit_expr_for_dyn_imports_ref<F: FnMut(&Expr)>(expr: &Expr, f: &mut F) {
if matches!(expr, Expr::DynamicImport { .. }) {
f(expr);
}
// Closure bodies — mirror the mutable visitor so the collect pass and the
// fill pass traverse dynamic import sites in the same order.
if let Expr::Closure { body, .. } = expr {
for s in body {
visit_stmt_for_dyn_imports_ref(s, f);
}
}
walk_expr_children(expr, &mut |child| visit_expr_for_dyn_imports_ref(child, f));
}

Expand Down
53 changes: 36 additions & 17 deletions crates/perry-hir/src/jsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use crate::lower::{lower_expr, LoweringContext};
pub(crate) fn lower_jsx_element(ctx: &mut LoweringContext, jsx: &ast::JSXElement) -> Result<Expr> {
let type_expr = lower_jsx_element_name(ctx, &jsx.opening.name)?;

let mut props_fields: Vec<(String, Expr)> = Vec::new();
let mut props_parts: Vec<(Option<String>, Expr)> = Vec::new();
let mut has_spread_attr = false;
for attr in &jsx.opening.attrs {
match attr {
ast::JSXAttrOrSpread::JSXAttr(jsx_attr) => {
Expand All @@ -31,12 +32,11 @@ pub(crate) fn lower_jsx_element(ctx: &mut LoweringContext, jsx: &ast::JSXElement
None => Expr::Bool(true), // Boolean attribute: <input disabled />
Some(val) => lower_jsx_attr_value(ctx, val)?,
};
props_fields.push((attr_name, attr_val));
props_parts.push((Some(attr_name), attr_val));
}
ast::JSXAttrOrSpread::SpreadElement(spread) => {
// Spread attributes ({...obj}) are not yet representable in HIR Object.
// Evaluate for side effects but don't propagate into props.
let _ = lower_expr(ctx, &spread.expr);
has_spread_attr = true;
props_parts.push((None, lower_expr(ctx, &spread.expr)?));
}
}
}
Expand All @@ -54,17 +54,24 @@ pub(crate) fn lower_jsx_element(ctx: &mut LoweringContext, jsx: &ast::JSXElement
match children.len() {
0 => {}
1 => {
props_fields.push(("children".to_string(), children.remove(0)));
props_parts.push((Some("children".to_string()), children.remove(0)));
}
_ => {
props_fields.push(("children".to_string(), Expr::Array(children)));
props_parts.push((Some("children".to_string()), Expr::Array(children)));
}
}

let props_expr = if props_fields.is_empty() {
let props_expr = if props_parts.is_empty() {
Expr::Null
} else if has_spread_attr {
Expr::ObjectSpread { parts: props_parts }
} else {
Expr::Object(props_fields)
Expr::Object(
props_parts
.into_iter()
.map(|(key, value)| (key.expect("non-spread JSX prop"), value))
.collect(),
)
};

// #4950: a module that default-imports the npm `react` package gets
Expand Down Expand Up @@ -176,9 +183,26 @@ pub(crate) fn lower_jsx_fragment(
}),
property: "Fragment".to_string(),
};
if let Some(call) = react_create_element_call(ctx, fragment_type, &props_expr) {
return Ok(call);
}
return Ok(Expr::Call {
callee: Box::new(Expr::PropertyGet {
object: Box::new(if let Some(id) = ctx.lookup_local(&react_local) {
Expr::LocalGet(id)
} else {
Expr::ExternFuncRef {
name: ctx
.lookup_imported_func(&react_local)
.unwrap_or(&react_local)
.to_string(),
param_types: Vec::new(),
return_type: Type::Any,
}
}),
property: "createElement".to_string(),
}),
args: vec![fragment_type, props_expr],
type_args: Vec::new(),
byte_offset: 0,
});
}

Ok(Expr::Call {
Expand All @@ -187,17 +211,12 @@ pub(crate) fn lower_jsx_fragment(
param_types: Vec::new(),
return_type: Type::Any,
}),
// Fragment marker: inline "__Fragment" string. perry-react's jsx() checks
// `type === "__Fragment"` to detect fragment elements.
args: vec![Expr::String("__Fragment".to_string()), props_expr],
type_args: Vec::new(),
byte_offset: 0,
})
}

/// Lower a JSX element name to an HIR expression.
/// Lowercase tag names (HTML intrinsics) become string literals.
/// Uppercase tag names (components) are looked up as identifiers.
pub(crate) fn lower_jsx_element_name(
ctx: &mut LoweringContext,
name: &ast::JSXElementName,
Expand Down
45 changes: 42 additions & 3 deletions crates/perry-hir/src/lower/expr_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ use crate::lower_patterns::{
generate_param_destructuring_stmts, get_param_default, get_pat_name, get_pat_type,
is_destructuring_pattern, is_rest_param,
};
use crate::lower_types::hoisted_text_codec::{
infer_hoisted_text_codec_var_type, require_literal_specifier,
};

use super::{lower_expr, LoweringContext};

Expand Down Expand Up @@ -735,6 +738,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul
// #4950: undefined-initialised `Stmt::Let`s for `var`s found nested in
// compound statements — prepended to the lowered body below.
let mut nested_var_prologue: Vec<Stmt> = Vec::new();
let mut top_level_var_prologue: Vec<Stmt> = Vec::new();
if let Some(ref block) = fn_expr.function.body {
// Issue #838 followup (b): pre-register top-level `var` decls in
// this function body BEFORE lowering any statement. dayjs's
Expand All @@ -752,6 +756,7 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul
// shared synthetic class id on the instance and dispatch finds
// the prototype methods. Same shallow-walk policy as the
// codegen-side `referenced_from_fn` pre-scan.
let mut builtin_aliases_in_var_decl = std::collections::HashSet::new();
for stmt in &block.stmts {
if let ast::Stmt::Decl(ast::Decl::Var(var_decl)) = stmt {
if var_decl.kind == ast::VarDeclKind::Var {
Expand All @@ -774,12 +779,41 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul
&decl.name, &mut names,
);
for name in names {
let ty = if let ast::Pat::Ident(ident) = &decl.name {
if decl.init.as_deref().and_then(require_literal_specifier)
== Some("util")
|| decl.init.as_deref().and_then(require_literal_specifier)
== Some("node:util")
{
builtin_aliases_in_var_decl.insert(name.clone());
}
infer_hoisted_text_codec_var_type(decl, ident, |name| {
builtin_aliases_in_var_decl.contains(name)
|| matches!(
ctx.lookup_builtin_module_alias(name),
Some("util" | "node:util")
)
|| matches!(
ctx.lookup_native_module(name),
Some(("util" | "node:util", None))
)
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
Type::Any
};
let already_in_scope = ctx
.locals
.lookup_index_in_scope(&name, outer_locals_len)
.is_some();
if !already_in_scope {
let id = ctx.define_local(name, Type::Any);
let id = ctx.define_local(name.clone(), ty.clone());
top_level_var_prologue.push(Stmt::Let {
id,
name,
ty,
mutable: true,
init: Some(Expr::Undefined),
});
// Mark as hoisted so closures created
// before the var's init expression see
// it through a box (mutable capture),
Expand Down Expand Up @@ -1090,8 +1124,13 @@ fn lower_fn_expr_anon(ctx: &mut LoweringContext, fn_expr: &ast::FnExpr) -> Resul
_ => exec_stmts.extend(lowered),
}
}
let mut combined: Vec<Stmt> =
Vec::with_capacity(nested_var_prologue.len() + func_decls.len() + exec_stmts.len());
let mut combined: Vec<Stmt> = Vec::with_capacity(
top_level_var_prologue.len()
+ nested_var_prologue.len()
+ func_decls.len()
+ exec_stmts.len(),
);
combined.extend(std::mem::take(&mut top_level_var_prologue));
// Nested-var undefined slots first so every later read/write —
// including from hoisted function-declaration closures — sees
// initialised storage (#4950).
Expand Down
97 changes: 92 additions & 5 deletions crates/perry-hir/src/lower/lower_module_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,77 @@ use swc_ecma_ast as ast;

use super::*;
use crate::ir::*;
use crate::lower_types::hoisted_text_codec::{
infer_hoisted_text_codec_var_type, require_literal_specifier,
};

fn should_enable_react_automatic_jsx(name: &str, ast_module: &ast::Module) -> bool {
let is_jsx_source = name.ends_with(".tsx")
|| name.ends_with(".jsx")
|| name.contains(".tsx?")
|| name.contains(".jsx?");
if !is_jsx_source {
return false;
}

let mut has_explicit_react_import = false;
let mut has_react_ecosystem_import = false;
for item in &ast_module.body {
let ast::ModuleItem::ModuleDecl(ast::ModuleDecl::Import(import)) = item else {
continue;
};
let source = import.src.value.to_string_lossy().to_string();
if source == "react" && import_has_runtime_binding(import) {
has_explicit_react_import = true;
}
Comment on lines +30 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n --glob '*.{rs,sh,ts,tsx,js,jsx}' \
  'import\s+type\b.*from\s+["'\''"]react["'\''"]|from\s+["'\''"]react["'\''"].*\btype\b' .

Repository: PerryTS/perry

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and inspect the lowering logic.
ast-grep outline crates/perry-hir/src/lower/lower_module_fn.rs --view expanded || true
wc -l crates/perry-hir/src/lower/lower_module_fn.rs
sed -n '1,120p' crates/perry-hir/src/lower/lower_module_fn.rs

# Inspect the AST import fields used by this code path.
rg -n "type_only|ImportSpecifier::Named|ImportSpecifier::Default|ImportSpecifier::Namespace|has_explicit_react_import|react_ecosystem" crates/perry-hir crates -g '*.rs'

Repository: PerryTS/perry

Length of output: 25056


Ignore type-only React imports here. import type ... from "react" still flips has_explicit_react_import, so TSX files with only React type imports skip the synthetic react namespace import and can fall back to the eager JSX path. Check import.type_only and named.is_type_only before treating React as available.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-hir/src/lower/lower_module_fn.rs` around lines 30 - 39, The
React import detection in lower_module_fn currently treats all imports from
"react" as runtime imports, so type-only React imports still set
has_explicit_react_import. Update the import scan in the lower_module_fn logic
to ignore type-only imports by checking import.type_only and any named
specifiers’ is_type_only flags before marking React as available, so only real
runtime React imports prevent the synthetic namespace import.

if source.starts_with("@tanstack/react-")
|| source == "@tanstack/react-router"
|| source == "react/jsx-runtime"
{
has_react_ecosystem_import = true;
}
}

if has_explicit_react_import {
return false;
}

has_react_ecosystem_import
|| name.contains("node_modules/@tanstack/react-")
|| name.contains("node_modules/@tanstack/react-router/")
}

fn import_has_runtime_binding(import: &ast::ImportDecl) -> bool {
if import.type_only {
return false;
}
import.specifiers.iter().any(|spec| match spec {
ast::ImportSpecifier::Named(named) => !named.is_type_only,
ast::ImportSpecifier::Default(_) | ast::ImportSpecifier::Namespace(_) => true,
})
}

fn enable_react_automatic_jsx(module: &mut Module, ctx: &mut LoweringContext) {
const LOCAL: &str = "__perry_react_auto";
let local = LOCAL.to_string();
ctx.register_imported_func(local.clone(), local.clone());
ctx.namespace_import_locals.insert(local.clone());
ctx.namespace_import_sources
.insert(local.clone(), "react".to_string());
ctx.react_default_import_local = Some(local.clone());
module.imports.push(Import {
source: "react".to_string(),
specifiers: vec![ImportSpecifier::Namespace { local }],
is_native: false,
module_kind: ModuleKind::NativeCompiled,
resolved_path: None,
type_only: false,
is_dynamic: false,
is_dynamic_target: false,
is_deferred_require: false,
is_adopted_require: false,
});
}

fn module_has_strict_mode(ast_module: &ast::Module) -> bool {
for item in &ast_module.body {
Expand Down Expand Up @@ -355,6 +426,9 @@ pub fn lower_module_full(
ctx.seed_imported_class_accessors(seed);
}
let mut module = Module::new(name);
if should_enable_react_automatic_jsx(name, ast_module) {
enable_react_automatic_jsx(&mut module, &mut ctx);
}

// Pre-scan for `new Function` / `Function(...)` constant-argument
// resolution: single-assignment module vars, `toString`-bearing object
Expand Down Expand Up @@ -536,6 +610,7 @@ pub fn lower_module_full(
_ => None,
};
if let Some(var_decl) = var_decl {
let mut builtin_aliases_in_decl = HashSet::new();
for decl in &var_decl.decls {
// #4461: `var X = class { ... }` is lowered as a class
// expression bound to the name `X` (see stmt.rs) — the class
Expand All @@ -551,12 +626,24 @@ pub fn lower_module_full(
}
if let ast::Pat::Ident(ident) = &decl.name {
let name = ident.id.sym.to_string();
if decl.init.as_deref().and_then(require_literal_specifier) == Some("util")
|| decl.init.as_deref().and_then(require_literal_specifier)
== Some("node:util")
{
builtin_aliases_in_decl.insert(name.clone());
}
if ctx.lookup_local(&name).is_none() {
let ty = ident
.type_ann
.as_ref()
.map(|ann| extract_ts_type(&ann.type_ann))
.unwrap_or(Type::Any);
let ty = infer_hoisted_text_codec_var_type(decl, ident, |name| {
builtin_aliases_in_decl.contains(name)
|| matches!(
ctx.lookup_builtin_module_alias(name),
Some("util" | "node:util")
)
|| matches!(
ctx.lookup_native_module(name),
Some(("util" | "node:util", None))
)
});
ctx.define_local(name.clone(), ty);
ctx.pre_registered_module_vars.insert(name);
if var_decl.kind == ast::VarDeclKind::Var {
Expand Down
11 changes: 10 additions & 1 deletion crates/perry-hir/src/lower/module_decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ pub(crate) fn lower_module_decl(
// so JSX in this module lowers to
// `<local>.createElement(...)` instead of Perry's
// eager `js_jsx` adapter (see jsx.rs).
if source == "react" {
if source == "react" && !whole_decl_type_only {
ctx.react_default_import_local = Some(local.clone());
}
}
Expand Down Expand Up @@ -422,6 +422,15 @@ pub(crate) fn lower_module_decl(
// not lower to StaticMethodCall — see the heuristic
// in expr_call::static_and_instance.
ctx.namespace_import_locals.insert(local.clone());
// React namespace imports are the common TSX shape
// (`import * as React from "react"`). They need the
// same non-eager React element semantics as default
// React imports; Perry's native JSX adapter calls
// function components immediately and therefore runs
// hooks outside the reconciler.
if source == "react" && !whole_decl_type_only {
ctx.react_default_import_local = Some(local.clone());
}
Comment on lines +425 to +433

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n --glob '*.{rs,sh,ts,tsx,js,jsx}' \
  'import\s+type\s+\*\s+as\s+\w+\s+from\s+["'\''"]react["'\''"]' .

Repository: PerryTS/perry

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant file and surrounding code.
FILE="crates/perry-hir/src/lower/module_decl.rs"

echo "== Outline =="
ast-grep outline "$FILE" --view expanded || true

echo
echo "== Relevant lines around 360-470 =="
sed -n '360,470p' "$FILE" | cat -n

echo
echo "== Search for whole_decl_type_only usage =="
rg -n 'whole_decl_type_only|react_default_import_local|namespace import|import type \* as' "$FILE" .

Repository: PerryTS/perry

Length of output: 50369


Exclude type-only React namespace imports from the runtime JSX binding. import type * as React from "react" is erased, but this branch still stores react_default_import_local, so JSX can lower to React.createElement(...) without a runtime React binding. Gate the React assignment on !whole_decl_type_only.

Proposed fix
-                            if source == "react" {
+                            if source == "react" && !whole_decl_type_only {
                                 ctx.react_default_import_local = Some(local.clone());
                             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// React namespace imports are the common TSX shape
// (`import * as React from "react"`). They need the
// same non-eager React element semantics as default
// React imports; Perry's native JSX adapter calls
// function components immediately and therefore runs
// hooks outside the reconciler.
if source == "react" {
ctx.react_default_import_local = Some(local.clone());
}
// React namespace imports are the common TSX shape
// (`import * as React from "react"`). They need the
// same non-eager React element semantics as default
// React imports; Perry's native JSX adapter calls
// function components immediately and therefore runs
// hooks outside the reconciler.
if source == "react" && !whole_decl_type_only {
ctx.react_default_import_local = Some(local.clone());
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-hir/src/lower/module_decl.rs` around lines 425 - 433, The React
namespace import handling in module_decl.rs is currently binding type-only
imports into `react_default_import_local`, which can leave JSX lowering pointing
at a runtime React symbol that does not exist. Update the `source == "react"`
branch in the import lowering logic to skip this assignment when
`whole_decl_type_only` is true, so only real runtime React namespace imports
populate `ctx.react_default_import_local`. Keep the existing behavior for
non-type-only React namespace imports and leave the JSX lowering path unchanged
otherwise.

// Remember the source so a later bare `export { local }`
// re-exports the namespace itself rather than a bare
// function symbol (see the local-export branch below).
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-hir/src/lower_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ const INFER_TYPE_RECURSION_CAP: u32 = 48;
const INFER_TYPE_STACK_RED_ZONE: usize = 256 * 1024;
const INFER_TYPE_STACK_SEGMENT: usize = 2 * 1024 * 1024;

pub(crate) mod hoisted_text_codec;

pub(crate) fn infer_type_from_expr(expr: &ast::Expr, ctx: &LoweringContext) -> Type {
thread_local! {
static INFER_DEPTH: std::cell::Cell<u32> = const { std::cell::Cell::new(0) };
Expand Down
Loading
Loading