Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(transformer/optional-chaining): avoid multiple symbol lookups #7421

Merged
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
51 changes: 29 additions & 22 deletions crates/oxc_transformer/src/es2020/optional_chaining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use std::mem;

use oxc_allocator::CloneIn;
use oxc_ast::{ast::*, NONE};
use oxc_semantic::{IsGlobalReference, SymbolFlags};
use oxc_semantic::SymbolFlags;
use oxc_span::SPAN;
use oxc_traverse::{Ancestor, BoundIdentifier, MaybeBoundIdentifier, Traverse, TraverseCtx};

Expand Down Expand Up @@ -180,17 +180,26 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
}
}

/// Check if we should create a temp variable for the identifier
/// Check if we should create a temp variable for the identifier.
///
/// Except for `eval`, we should create a temp variable for all global references
fn should_create_temp_variable_for_identifier(
/// Except for `eval`, we should create a temp variable for all global references.
///
/// If no temp variable required, returns `MaybeBoundIdentifier` for existing variable/global.
/// If temp variable is required, returns `None`.
fn get_existing_binding_for_identifier(
&self,
ident: &IdentifierReference<'a>,
ctx: &TraverseCtx<'a>,
) -> bool {
!self.ctx.assumptions.pure_getters
&& ident.is_global_reference(ctx.symbols())
&& ident.name != "eval"
) -> Option<MaybeBoundIdentifier<'a>> {
let binding = MaybeBoundIdentifier::from_identifier_reference(ident, ctx);
if self.ctx.assumptions.pure_getters
|| binding.to_bound_identifier().is_some()
|| ident.name == "eval"
{
Some(binding)
} else {
None
}
}

/// Return `left === null`
Expand Down Expand Up @@ -544,8 +553,7 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
// If the expression is an identifier and it's not a global reference, we just wrap it with checks
// `foo` -> `foo === null || foo === void 0`
if let Expression::Identifier(ident) = expr {
if !self.should_create_temp_variable_for_identifier(ident, ctx) {
let binding = MaybeBoundIdentifier::from_identifier_reference(ident, ctx);
if let Some(binding) = self.get_existing_binding_for_identifier(ident, ctx) {
let left1 = binding.create_read_expression(ctx);
let left2 = binding.create_read_expression(ctx);
if ident.name == "eval" {
Expand All @@ -568,18 +576,17 @@ impl<'a, 'ctx> OptionalChaining<'a, 'ctx> {
// If the [`MemberExpression::object`] is a global reference, we need to assign it to a temp binding.
// i.e `foo` -> `(_foo = foo)`
if let Expression::Identifier(ident) = object {
let binding = if self.should_create_temp_variable_for_identifier(ident, ctx) {
let binding = self.generate_binding(object, ctx);
// `(_foo = foo)`
*object = Self::create_assignment_expression(
binding.create_write_target(ctx),
ctx.ast.move_expression(object),
ctx,
);
binding.to_maybe_bound_identifier()
} else {
MaybeBoundIdentifier::from_identifier_reference(ident, ctx)
};
let binding =
self.get_existing_binding_for_identifier(ident, ctx).unwrap_or_else(|| {
let binding = self.generate_binding(object, ctx);
// `(_foo = foo)`
*object = Self::create_assignment_expression(
binding.create_write_target(ctx),
ctx.ast.move_expression(object),
ctx,
);
binding.to_maybe_bound_identifier()
});
self.set_binding_context(binding);
} else if matches!(object, Expression::Super(_)) {
self.set_this_context();
Expand Down
Loading