Skip to content

Commit

Permalink
fix(transformer/react): don't transform if the variable does not have…
Browse files Browse the repository at this point in the history
… a value reference
  • Loading branch information
Dunqing committed Sep 9, 2024
1 parent e38114b commit 5520055
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 30 deletions.
5 changes: 5 additions & 0 deletions crates/oxc_semantic/src/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ impl Reference {
self.flags.is_write()
}

/// Returns `true` if this reference is used in a value context.
pub fn is_value(&self) -> bool {
self.flags.is_value()
}

/// Returns `true` if this reference is used in a type context.
#[inline]
pub fn is_type(&self) -> bool {
Expand Down
61 changes: 35 additions & 26 deletions crates/oxc_transformer/src/react/refresh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{cell::Cell, iter::once};

use oxc_allocator::CloneIn;
use oxc_ast::{ast::*, match_expression, match_member_expression};
use oxc_semantic::{ReferenceFlags, ScopeId, SymbolFlags, SymbolId};
use oxc_semantic::{Reference, ReferenceFlags, ScopeId, SymbolFlags, SymbolId};
use oxc_span::{Atom, GetSpan, SPAN};
use oxc_syntax::operator::AssignmentOperator;
use oxc_traverse::{Ancestor, Traverse, TraverseCtx};
Expand Down Expand Up @@ -491,12 +491,14 @@ impl<'a> ReactRefresh<'a> {
}
}

*expr = ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
self.create_registration(ctx.ast.atom(inferred_name), ctx),
ctx.ast.move_expression(expr),
);
if !is_variable_declarator {
*expr = ctx.ast.expression_assignment(
SPAN,
AssignmentOperator::Assign,
self.create_registration(ctx.ast.atom(inferred_name), ctx),
ctx.ast.move_expression(expr),
);
}

true
}
Expand Down Expand Up @@ -756,6 +758,7 @@ impl<'a> ReactRefresh<'a> {
let declarator = decl.declarations.first_mut().unwrap_or_else(|| unreachable!());
let init = declarator.init.as_mut()?;
let id = declarator.id.get_binding_identifier()?;
let symbol_id = id.symbol_id.get()?;

if !is_componentish_name(&id.name) {
return None;
Expand All @@ -782,33 +785,39 @@ impl<'a> ReactRefresh<'a> {
// babel-plugin-styled-components)
}
Expression::CallExpression(call_expr) => {
if matches!(call_expr.callee, Expression::ImportExpression(_))
|| call_expr.is_require_call()
{
let is_import_expression = match call_expr.callee.get_inner_expression() {
Expression::ImportExpression(_) => {
true
}
Expression::Identifier(ident) => {
ident.name.starts_with("require")
},
_ => false
};

if is_import_expression {
return None;
}

// Maybe a HOC.
// Try to determine if this is some form of import.
let found_inside = self.replace_inner_components(
&id.name,
init,
/* is_variable_declarator */ true,
ctx,
);
if !found_inside {
return None;
}

// See if this identifier is used in JSX. Then it's a component.
// TODO:
// https://github.com/facebook/react/blob/ba6a9e94edf0db3ad96432804f9931ce9dc89fec/packages/react-refresh/src/ReactFreshBabelPlugin.js#L161-L199
}
_ => {
return None;
}
}

// Maybe a HOC.
// Try to determine if this is some form of import.
let found_inside = self
.replace_inner_components(&id.name, init, /* is_variable_declarator */ true, ctx);

if !found_inside {
// See if this identifier is used in JSX. Then it's a component.
// TODO: Here we should check if the variable is used in JSX. But now we only check if it has value references.
// https://github.com/facebook/react/blob/ba6a9e94edf0db3ad96432804f9931ce9dc89fec/packages/react-refresh/src/ReactFreshBabelPlugin.js#L161-L199
if !ctx.symbols().get_resolved_references(symbol_id).any(Reference::is_value) {
return None;
}
}

Some(self.create_assignment_expression(id, ctx))
}

Expand Down
53 changes: 50 additions & 3 deletions tasks/transform_conformance/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 3bcfee23

Passed: 17/49
Passed: 17/50

# All Passed:
* babel-plugin-transform-nullish-coalescing-operator
Expand Down Expand Up @@ -167,7 +167,7 @@ rebuilt : SymbolId(2): []
x Output mismatch


# babel-plugin-transform-react-jsx (3/27)
# babel-plugin-transform-react-jsx (3/28)
* refresh/can-handle-implicit-arrow-returns/input.jsx
Symbol reference IDs mismatch:
after transform: SymbolId(9): [ReferenceId(23), ReferenceId(24), ReferenceId(25)]
Expand Down Expand Up @@ -265,6 +265,9 @@ Unresolved references mismatch:
after transform: ["item", "useFoo"]
rebuilt : ["$RefreshSig$", "item", "useFoo"]

* refresh/does-not-transform-it-because-it-is-not-used-in-the-AST/input.jsx
x Output mismatch

* refresh/generates-signatures-for-function-declarations-calling-hooks/input.jsx
x Output mismatch

Expand All @@ -290,7 +293,51 @@ x Output mismatch
x Output mismatch

* refresh/registers-identifiers-used-in-react-create-element-at-definition-site/input.jsx
x Output mismatch
Symbol reference IDs mismatch:
after transform: SymbolId(13): [ReferenceId(33), ReferenceId(47), ReferenceId(48)]
rebuilt : SymbolId(13): [ReferenceId(2), ReferenceId(48)]
Symbol reference IDs mismatch:
after transform: SymbolId(14): [ReferenceId(35), ReferenceId(49), ReferenceId(50)]
rebuilt : SymbolId(14): [ReferenceId(5), ReferenceId(50)]
Symbol reference IDs mismatch:
after transform: SymbolId(15): [ReferenceId(37), ReferenceId(51), ReferenceId(52)]
rebuilt : SymbolId(15): [ReferenceId(8), ReferenceId(52)]
Symbol reference IDs mismatch:
after transform: SymbolId(16): [ReferenceId(39), ReferenceId(53), ReferenceId(54)]
rebuilt : SymbolId(16): [ReferenceId(12), ReferenceId(54)]
Symbol reference IDs mismatch:
after transform: SymbolId(17): [ReferenceId(41), ReferenceId(55), ReferenceId(56)]
rebuilt : SymbolId(17): [ReferenceId(35), ReferenceId(56)]
Symbol reference IDs mismatch:
after transform: SymbolId(18): [ReferenceId(43), ReferenceId(57), ReferenceId(58)]
rebuilt : SymbolId(18): [ReferenceId(41), ReferenceId(58)]
Symbol reference IDs mismatch:
after transform: SymbolId(19): [ReferenceId(45), ReferenceId(59), ReferenceId(60)]
rebuilt : SymbolId(19): [ReferenceId(45), ReferenceId(60)]
Reference symbol mismatch:
after transform: ReferenceId(47): Some("_c")
rebuilt : ReferenceId(47): None
Reference symbol mismatch:
after transform: ReferenceId(49): Some("_c2")
rebuilt : ReferenceId(49): None
Reference symbol mismatch:
after transform: ReferenceId(51): Some("_c3")
rebuilt : ReferenceId(51): None
Reference symbol mismatch:
after transform: ReferenceId(53): Some("_c4")
rebuilt : ReferenceId(53): None
Reference symbol mismatch:
after transform: ReferenceId(55): Some("_c5")
rebuilt : ReferenceId(55): None
Reference symbol mismatch:
after transform: ReferenceId(57): Some("_c6")
rebuilt : ReferenceId(57): None
Reference symbol mismatch:
after transform: ReferenceId(59): Some("_c7")
rebuilt : ReferenceId(59): None
Unresolved references mismatch:
after transform: ["React", "funny", "hoc", "jsx", "styled", "wow"]
rebuilt : ["$RefreshReg$", "React", "funny", "hoc", "jsx", "styled", "wow"]

* refresh/registers-likely-hocs-with-inline-functions-1/input.jsx
x Output mismatch
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const StyledFactory1 = styled('div')`color: hotpink`

console.log(StyledFactory1);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const StyledFactory1 = styled('div')`color: hotpink`
console.log(StyledFactory1);
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function Foo() {
React.createElement(Alias1),
React.createElement(Alias2),
jsx(Header),
React.createElement(Dict.X),
React.createElement(Dict.X)
];
}

Expand Down

0 comments on commit 5520055

Please sign in to comment.