Skip to content
Closed
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
2 changes: 2 additions & 0 deletions crates/swc_ecma_react_compiler/src/convert_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::preserved_ast::PreservedAst;
pub struct ConvertResult {
pub file: File,
pub preserved_ast: PreservedAst,
pub source_file_start_pos: swc_common::BytePos,
}

/// Converts an SWC AST to the React compiler's Babel-compatible AST.
Expand All @@ -44,6 +45,7 @@ pub fn convert_program(
ConvertResult {
file,
preserved_ast: ctx.preserved_ast.into_inner(),
source_file_start_pos: program.span().lo,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the file start for loc-index recovery

This value is later added to Babel loc.index values to recover SWC spans, but program.span().lo is the first parsed token rather than the beginning of the source file. For any input with leading whitespace or comments before the first token, a compiler-created node that has only loc will be shifted forward by that leading length (for example, loc.index == 0 maps to the first token instead of BytePos(1)), so emitted sourcemaps point at the wrong original text. Pass the actual file start, or derive it from the program base location, instead.

Useful? React with 👍 / 👎.

}
}

Expand Down
109 changes: 105 additions & 4 deletions crates/swc_ecma_react_compiler/src/convert_ast_reverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,25 @@ use swc_ecma_ast as swc;
use crate::preserved_ast::PreservedAst;

/// Convert with source text and preserved SWC nodes from the forward pass.
pub fn convert_program_to_swc(file: &File, preserved_ast: PreservedAst) -> swc::Program {
let ctx = ReverseCtx::new(preserved_ast);
pub fn convert_program_to_swc(
file: &File,
preserved_ast: PreservedAst,
source_file_start_pos: BytePos,
) -> swc::Program {
let ctx = ReverseCtx::new(preserved_ast, source_file_start_pos);
ctx.convert_program(&file.program)
}

struct ReverseCtx {
preserved_ast: RefCell<PreservedAst>,
source_file_start_pos: BytePos,
}

impl ReverseCtx {
fn new(preserved_ast: PreservedAst) -> Self {
fn new(preserved_ast: PreservedAst, source_file_start_pos: BytePos) -> Self {
Self {
preserved_ast: RefCell::new(preserved_ast),
source_file_start_pos,
}
}

Expand Down Expand Up @@ -103,10 +109,27 @@ impl ReverseCtx {
}

fn span_from_base(&self, base: &BaseNode) -> Span {
// `start` and `end` preserve absolute SWC `BytePos` values from the
// forward conversion, while `loc` indices are file-relative.
match (base.start, base.end) {
(Some(start), Some(end)) => Span::new(BytePos(start), BytePos(end)),
(Some(start), None) => Span::new(BytePos(start), BytePos(start)),
_ => DUMMY_SP,
_ => base.loc.as_ref().map_or(DUMMY_SP, |loc| {
let start = loc
.start
.index
.map(|index| self.source_file_start_pos + BytePos(index));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Normalize loc.index before using it for shared SourceMaps

When callers parse multiple files with one SourceMap, this fallback still produces wrong spans because the forward converter builds loc.index from the absolute SWC BytePos and clamps it to source_text.len() instead of subtracting the source file's start. For every file after the first, React-compiler-created nodes that only have loc can therefore recover EOF-ish or out-of-file positions when this line adds the source-file base, so the sourcemap entries are skipped or point at the wrong text. Make the forward loc.index file-relative before relying on it here.

Useful? React with 👍 / 👎.

let end = loc
.end
.index
.or(loc.start.index)
.map(|index| self.source_file_start_pos + BytePos(index));

match (start, end) {
(Some(start), Some(end)) => Span::new(start, end),
_ => DUMMY_SP,
}
}),
}
}

Expand Down Expand Up @@ -2916,3 +2939,81 @@ fn ts_type_operator_op(operator: &str) -> Option<swc::TsTypeOperatorOp> {
_ => None,
}
}

#[cfg(test)]
mod tests {
use react_compiler_ast::common::{Position, SourceLocation};

use super::*;

fn base(
start: Option<u32>,
end: Option<u32>,
loc_start: Option<u32>,
loc_end: Option<u32>,
) -> BaseNode {
BaseNode {
start,
end,
loc: loc_start.map(|index| SourceLocation {
start: Position {
line: 1,
column: index,
index: Some(index),
},
end: Position {
line: 1,
column: loc_end.unwrap_or_default(),
index: loc_end,
},
filename: None,
identifier_name: None,
}),
..Default::default()
}
}

fn ctx() -> ReverseCtx {
ReverseCtx::new(Default::default(), BytePos(1_000))
}

#[test]
fn span_from_base_preserves_start_and_end() {
assert_eq!(
ctx().span_from_base(&base(Some(10), Some(20), Some(1), Some(2))),
Span::new(BytePos(10), BytePos(20))
);
}

#[test]
fn span_from_base_collapses_partial_start_and_end() {
assert_eq!(
ctx().span_from_base(&base(Some(10), None, Some(1), Some(2))),
Span::new(BytePos(10), BytePos(10))
);
}

#[test]
fn span_from_base_uses_file_relative_loc_indices() {
assert_eq!(
ctx().span_from_base(&base(None, None, Some(10), Some(20))),
Span::new(BytePos(1_010), BytePos(1_020))
);
}

#[test]
fn span_from_base_collapses_partial_loc_indices() {
assert_eq!(
ctx().span_from_base(&base(None, None, Some(10), None)),
Span::new(BytePos(1_010), BytePos(1_010))
);
}

#[test]
fn span_from_base_without_positions_is_dummy() {
assert_eq!(
ctx().span_from_base(&base(None, None, None, None)),
DUMMY_SP
);
}
}
3 changes: 2 additions & 1 deletion crates/swc_ecma_react_compiler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub fn transform(
let ConvertResult {
file,
preserved_ast,
source_file_start_pos,
} = convert_program(program, source_text, comments);
let emit_success_error_diagnostics = options.no_emit;
let result =
Expand All @@ -117,7 +118,7 @@ pub fn transform(

let rename_plan = build_rename_plan(&scope_info, &renames);
let program = file.map(|file: react_compiler_ast::File| {
let mut compiled = convert_program_to_swc(&file, preserved_ast);
let mut compiled = convert_program_to_swc(&file, preserved_ast, source_file_start_pos);
apply_renames(&mut compiled, &rename_plan);
compiled
});
Expand Down
142 changes: 139 additions & 3 deletions crates/swc_ecma_react_compiler/src/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

use std::collections::HashSet;

use react_compiler::entrypoint::plugin_options::{CompilerTarget, PluginOptions};
use react_compiler_ast::{
scope::{BindingId, BindingKind, ScopeInfo, ScopeKind},
statements::Statement,
File,
};
use swc_common::{sync::Lrc, FileName, SourceMap};
use swc_common::{sync::Lrc, BytePos, FileName, SourceMap};
use swc_ecma_ast::EsVersion;
use swc_ecma_codegen::Node;
use swc_ecma_parser::{parse_file_as_module, parse_file_as_program, EsSyntax, Syntax};
Expand All @@ -22,7 +24,16 @@ use crate::{
};

fn convert_program_to_swc(file: &File) -> swc_ecma_ast::Program {
convert_program_to_swc_with_preserved_ast(file, Default::default())
convert_program_to_swc_with_preserved_ast(
file,
Default::default(),
BytePos(
file.program
.base
.start
.expect("forward-converted program has a start"),
),
)
}

fn convert_module(module: &swc_ecma_ast::Module, source_text: &str) -> File {
Expand Down Expand Up @@ -1762,7 +1773,11 @@ fn convert_ts_source(source: &str) -> crate::convert_ast::ConvertResult {
fn round_trip_convert_result(result: crate::convert_ast::ConvertResult) -> swc_ecma_ast::Program {
assert_file_serializes_to_json(&result.file);

convert_program_to_swc_with_preserved_ast(&result.file, result.preserved_ast)
convert_program_to_swc_with_preserved_ast(
&result.file,
result.preserved_ast,
result.source_file_start_pos,
)
}

/// TS module-interop statements (`import x = require(...)`, `export = x`,
Expand Down Expand Up @@ -1926,3 +1941,124 @@ fn emit_program(program: &swc_ecma_ast::Program) -> Result<String, String> {

String::from_utf8(buf).map_err(|err| format!("emitted output is not valid UTF-8: {err}"))
}

#[test]
fn react_compiler_recovers_source_map_density_from_locations() {
let source = r#"
import { useCallback, useMemo, useState } from "react";

export function App({ items, onSelect }) {
const [selected, setSelected] = useState(null);
const activeItems = useMemo(
() => items.filter((item) => item.active),
[items],
);
const labels = useMemo(
() => activeItems.map((item) => `${item.name}:${item.id}`),
[activeItems],
);
const visibleLabels = useMemo(
() => labels.filter((label) => label.includes(":")),
[labels],
);
const selectItem = useCallback(
(item) => {
setSelected(item.id);
onSelect(item.id);
},
[onSelect],
);
const resetSelection = useCallback(() => setSelected(null), []);

return (
<section>
<button onClick={resetSelection}>Reset</button>
{activeItems.map((item, index) => (
<button
key={item.id}
onClick={() => selectItem(item)}
data-selected={selected === item.id}
>
{visibleLabels[index]}
</button>
))}
</section>
);
}
"#;
let cm = Lrc::new(SourceMap::default());
let file = cm.new_source_file(Lrc::new(FileName::Anon), source.to_string());
let mut errors = vec![];
let program = parse_file_as_program(
&file,
Syntax::Es(EsSyntax {
jsx: true,
..Default::default()
}),
EsVersion::latest(),
None,
&mut errors,
)
.expect("should parse");
let result = crate::transform(
&program,
SourceType::module(),
source,
None,
default_options(),
);
assert!(
result.diagnostics.is_empty(),
"unexpected compiler diagnostics: {:#?}",
result.diagnostics
);
let compiled = result.program.expect("component should compile");

let mut code = Vec::new();
let mut mappings = Vec::new();
{
let writer = swc_ecma_codegen::text_writer::JsWriter::new(
cm.clone(),
"\n",
&mut code,
Some(&mut mappings),
);
let mut emitter = swc_ecma_codegen::Emitter {
cfg: swc_ecma_codegen::Config::default(),
cm: cm.clone(),
comments: None,
wr: Box::new(writer),
};
compiled
.emit_with(&mut emitter)
.expect("should emit compiled program");
}

let emitted_lines = std::str::from_utf8(&code)
.expect("emitted output is valid UTF-8")
.lines()
.count()
.max(1);

let generated_lines = mappings
.iter()
.map(|(_, location)| location.line)
.collect::<HashSet<_>>();

// Ensure mappings are not concentrated on a small subset of generated lines.
assert!(
generated_lines.len() * 2 >= emitted_lines,
"expected mappings across most generated lines, got {} mapped lines out of {}",
generated_lines.len(),
emitted_lines
);

// Ensure we have more than one mapping per mapped line (helps catch regressions
// that collapse many node spans to DUMMY_SP).
assert!(
mappings.len() >= generated_lines.len() * 2,
"expected dense source map, got {} mappings across {} mapped lines",
mappings.len(),
generated_lines.len()
);
}
Loading