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 6, 2024
1 parent ff88c1f commit ea71b22
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 77 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
132 changes: 82 additions & 50 deletions tasks/transform_conformance/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 3bcfee23

Passed: 21/49
Passed: 21/50

# All Passed:
* babel-plugin-transform-nullish-coalescing-operator
Expand Down Expand Up @@ -231,7 +231,7 @@ Passed: 21/49



# babel-plugin-transform-react-jsx (6/27)
# babel-plugin-transform-react-jsx (6/28)
* refresh/can-handle-implicit-arrow-returns/input.jsx
x Symbol reference IDs mismatch:
| after transform: SymbolId(9): [ReferenceId(23), ReferenceId(24),
Expand Down Expand Up @@ -385,6 +385,22 @@ Passed: 21/49
| rebuilt : ["$RefreshSig$", "item", "useFoo"]


* refresh/does-not-transform-it-because-it-is-not-used-in-the-AST./input.jsx
x Output mismatch
x Symbol reference IDs mismatch:
| after transform: SymbolId(1): [ReferenceId(3), ReferenceId(5),
| ReferenceId(6)]
| rebuilt : SymbolId(1): [ReferenceId(1), ReferenceId(6)]

x Reference symbol mismatch:
| after transform: ReferenceId(5): Some("_c")
| rebuilt : ReferenceId(5): None

x Unresolved references mismatch:
| after transform: ["console", "styled"]
| rebuilt : ["$RefreshReg$", "console", "styled"]


* refresh/generates-signatures-for-function-declarations-calling-hooks/input.jsx
x Symbol reference IDs mismatch:
| after transform: SymbolId(5): [ReferenceId(6), ReferenceId(7),
Expand Down Expand Up @@ -653,122 +669,138 @@ Passed: 21/49


* refresh/registers-identifiers-used-in-jsx-at-definition-site/input.jsx
x Output mismatch
x Symbol reference IDs mismatch:
| after transform: SymbolId(13): [ReferenceId(22), ReferenceId(44),
| ReferenceId(45)]
| rebuilt : SymbolId(15): [ReferenceId(2), ReferenceId(45)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(14): [ReferenceId(24), ReferenceId(46),
| after transform: SymbolId(13): [ReferenceId(22), ReferenceId(46),
| ReferenceId(47)]
| rebuilt : SymbolId(16): [ReferenceId(5), ReferenceId(47)]
| rebuilt : SymbolId(15): [ReferenceId(2), ReferenceId(47)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(15): [ReferenceId(26), ReferenceId(48),
| after transform: SymbolId(14): [ReferenceId(24), ReferenceId(48),
| ReferenceId(49)]
| rebuilt : SymbolId(17): [ReferenceId(11), ReferenceId(49)]
| rebuilt : SymbolId(16): [ReferenceId(5), ReferenceId(49)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(16): [ReferenceId(28), ReferenceId(50),
| after transform: SymbolId(15): [ReferenceId(26), ReferenceId(50),
| ReferenceId(51)]
| rebuilt : SymbolId(18): [ReferenceId(34), ReferenceId(51)]
| rebuilt : SymbolId(17): [ReferenceId(8), ReferenceId(51)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(17): [ReferenceId(30), ReferenceId(52),
| after transform: SymbolId(16): [ReferenceId(28), ReferenceId(52),
| ReferenceId(53)]
| rebuilt : SymbolId(19): [ReferenceId(38), ReferenceId(53)]
| rebuilt : SymbolId(18): [ReferenceId(12), ReferenceId(53)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(18): [ReferenceId(32), ReferenceId(54),
| after transform: SymbolId(17): [ReferenceId(30), ReferenceId(54),
| ReferenceId(55)]
| rebuilt : SymbolId(20): [ReferenceId(42), ReferenceId(55)]
| rebuilt : SymbolId(19): [ReferenceId(36), ReferenceId(55)]

x Reference symbol mismatch:
| after transform: ReferenceId(44): Some("_c")
| rebuilt : ReferenceId(44): None
x Symbol reference IDs mismatch:
| after transform: SymbolId(18): [ReferenceId(32), ReferenceId(56),
| ReferenceId(57)]
| rebuilt : SymbolId(20): [ReferenceId(40), ReferenceId(57)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(19): [ReferenceId(34), ReferenceId(58),
| ReferenceId(59)]
| rebuilt : SymbolId(21): [ReferenceId(44), ReferenceId(59)]

x Reference symbol mismatch:
| after transform: ReferenceId(46): Some("_c2")
| after transform: ReferenceId(46): Some("_c")
| rebuilt : ReferenceId(46): None

x Reference symbol mismatch:
| after transform: ReferenceId(48): Some("_c3")
| after transform: ReferenceId(48): Some("_c2")
| rebuilt : ReferenceId(48): None

x Reference symbol mismatch:
| after transform: ReferenceId(50): Some("_c4")
| after transform: ReferenceId(50): Some("_c3")
| rebuilt : ReferenceId(50): None

x Reference symbol mismatch:
| after transform: ReferenceId(52): Some("_c5")
| after transform: ReferenceId(52): Some("_c4")
| rebuilt : ReferenceId(52): None

x Reference symbol mismatch:
| after transform: ReferenceId(54): Some("_c6")
| after transform: ReferenceId(54): Some("_c5")
| rebuilt : ReferenceId(54): None

x Reference symbol mismatch:
| after transform: ReferenceId(56): Some("_c6")
| rebuilt : ReferenceId(56): None

x Reference symbol mismatch:
| after transform: ReferenceId(58): Some("_c7")
| rebuilt : ReferenceId(58): None

x Unresolved references mismatch:
| after transform: ["funny", "hoc", "styled", "wow"]
| rebuilt : ["$RefreshReg$", "funny", "hoc", "styled", "wow"]


* refresh/registers-identifiers-used-in-react-create-element-at-definition-site/input.jsx
x Output mismatch
x Symbol reference IDs mismatch:
| after transform: SymbolId(13): [ReferenceId(33), ReferenceId(45),
| ReferenceId(46)]
| rebuilt : SymbolId(13): [ReferenceId(2), ReferenceId(46)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(14): [ReferenceId(35), ReferenceId(47),
| after transform: SymbolId(13): [ReferenceId(33), ReferenceId(47),
| ReferenceId(48)]
| rebuilt : SymbolId(14): [ReferenceId(5), ReferenceId(48)]
| rebuilt : SymbolId(13): [ReferenceId(2), ReferenceId(48)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(15): [ReferenceId(37), ReferenceId(49),
| after transform: SymbolId(14): [ReferenceId(35), ReferenceId(49),
| ReferenceId(50)]
| rebuilt : SymbolId(15): [ReferenceId(11), ReferenceId(50)]
| rebuilt : SymbolId(14): [ReferenceId(5), ReferenceId(50)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(16): [ReferenceId(39), ReferenceId(51),
| after transform: SymbolId(15): [ReferenceId(37), ReferenceId(51),
| ReferenceId(52)]
| rebuilt : SymbolId(16): [ReferenceId(33), ReferenceId(52)]
| rebuilt : SymbolId(15): [ReferenceId(8), ReferenceId(52)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(17): [ReferenceId(41), ReferenceId(53),
| after transform: SymbolId(16): [ReferenceId(39), ReferenceId(53),
| ReferenceId(54)]
| rebuilt : SymbolId(17): [ReferenceId(39), ReferenceId(54)]
| rebuilt : SymbolId(16): [ReferenceId(12), ReferenceId(54)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(18): [ReferenceId(43), ReferenceId(55),
| after transform: SymbolId(17): [ReferenceId(41), ReferenceId(55),
| ReferenceId(56)]
| rebuilt : SymbolId(18): [ReferenceId(43), ReferenceId(56)]
| rebuilt : SymbolId(17): [ReferenceId(35), ReferenceId(56)]

x Reference symbol mismatch:
| after transform: ReferenceId(45): Some("_c")
| rebuilt : ReferenceId(45): None
x Symbol reference IDs mismatch:
| after transform: SymbolId(18): [ReferenceId(43), ReferenceId(57),
| ReferenceId(58)]
| rebuilt : SymbolId(18): [ReferenceId(41), ReferenceId(58)]

x Symbol reference IDs mismatch:
| after transform: SymbolId(19): [ReferenceId(45), ReferenceId(59),
| ReferenceId(60)]
| rebuilt : SymbolId(19): [ReferenceId(45), ReferenceId(60)]

x Reference symbol mismatch:
| after transform: ReferenceId(47): Some("_c2")
| after transform: ReferenceId(47): Some("_c")
| rebuilt : ReferenceId(47): None

x Reference symbol mismatch:
| after transform: ReferenceId(49): Some("_c3")
| after transform: ReferenceId(49): Some("_c2")
| rebuilt : ReferenceId(49): None

x Reference symbol mismatch:
| after transform: ReferenceId(51): Some("_c4")
| after transform: ReferenceId(51): Some("_c3")
| rebuilt : ReferenceId(51): None

x Reference symbol mismatch:
| after transform: ReferenceId(53): Some("_c5")
| after transform: ReferenceId(53): Some("_c4")
| rebuilt : ReferenceId(53): None

x Reference symbol mismatch:
| after transform: ReferenceId(55): Some("_c6")
| after transform: ReferenceId(55): Some("_c5")
| rebuilt : ReferenceId(55): None

x Reference symbol mismatch:
| after transform: ReferenceId(57): Some("_c6")
| rebuilt : ReferenceId(57): None

x Reference symbol mismatch:
| after transform: ReferenceId(59): Some("_c7")
| rebuilt : ReferenceId(59): None

x Unresolved references mismatch:
| after transform: ["React", "funny", "hoc", "jsx", "styled", "wow"]
| rebuilt : ["$RefreshReg$", "React", "funny", "hoc", "jsx",
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 ea71b22

Please sign in to comment.