Skip to content

Commit 799e927

Browse files
authored
revert: "perf: napi source map serialize and deserialize (#10989)" (#11002)
Revert "perf: napi source map serialize and deserialize (#10989)" This reverts commit 9101e5d.
1 parent 738b79c commit 799e927

File tree

5 files changed

+47
-97
lines changed

5 files changed

+47
-97
lines changed

crates/node_binding/binding.d.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ export interface JsLoaderContext {
919919
content: null | Buffer
920920
additionalData?: any
921921
__internal__parseMeta: Record<string, string>
922-
sourceMap?: SourceMap
922+
sourceMap?: Buffer
923923
cacheable: boolean
924924
fileDependencies: Array<string>
925925
contextDependencies: Array<string>
@@ -2723,16 +2723,6 @@ export interface RegisterJsTaps {
27232723
registerRsdoctorPluginAssetsTaps: (stages: Array<number>) => Array<{ function: ((arg: JsRsdoctorAssetPatch) => Promise<boolean | undefined>); stage: number; }>
27242724
}
27252725

2726-
export interface SourceMap {
2727-
file?: string
2728-
sources?: Array<string | undefined | null>
2729-
sourceRoot?: string
2730-
sourcesContent?: Array<string | undefined | null>
2731-
names?: Array<string | undefined | null>
2732-
mappings?: string
2733-
debugId?: string
2734-
}
2735-
27362726
export interface SourceMapDevToolPluginOptions {
27372727
append?: (false | null) | string | Function
27382728
columns?: boolean

crates/rspack_binding_api/src/plugins/js_loader/context.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ use std::{collections::HashMap, ptr::NonNull, sync::Arc};
33
use napi::bindgen_prelude::*;
44
use napi_derive::napi;
55
use rspack_core::{LoaderContext, Module, RunnerContext};
6+
use rspack_error::ToStringResultToRspackResultExt;
67
use rspack_loader_runner::State as LoaderState;
78
use rspack_napi::threadsafe_js_value_ref::ThreadsafeJsValueRef;
89

9-
use crate::{ModuleObject, RspackError, SourceMap};
10+
use crate::{ModuleObject, RspackError};
1011

1112
#[napi(object)]
1213
#[derive(Hash)]
@@ -99,7 +100,7 @@ pub struct JsLoaderContext {
99100
pub additional_data: Option<ThreadsafeJsValueRef<Unknown<'static>>>,
100101
#[napi(js_name = "__internal__parseMeta")]
101102
pub parse_meta: HashMap<String, String>,
102-
pub source_map: Option<SourceMap>,
103+
pub source_map: Option<Buffer>,
103104
pub cacheable: bool,
104105
pub file_dependencies: Vec<String>,
105106
pub context_dependencies: Vec<String>,
@@ -144,7 +145,13 @@ impl TryFrom<&mut LoaderContext<RunnerContext>> for JsLoaderContext {
144145
.additional_data()
145146
.and_then(|data| data.get::<ThreadsafeJsValueRef<Unknown>>())
146147
.cloned(),
147-
source_map: cx.source_map().map(Into::into),
148+
source_map: cx
149+
.source_map()
150+
.cloned()
151+
.map(|v| v.to_json())
152+
.transpose()
153+
.to_rspack_result()?
154+
.map(|v| v.into_bytes().into()),
148155
cacheable: cx.cacheable,
149156
file_dependencies: cx
150157
.file_dependencies

crates/rspack_binding_api/src/plugins/js_loader/scheduler.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rspack_core::{
33
diagnostics::CapturedLoaderError, AdditionalData, LoaderContext, NormalModuleLoaderShouldYield,
44
NormalModuleLoaderStartYielding, RunnerContext, BUILTIN_LOADER_PREFIX,
55
};
6-
use rspack_error::{miette::IntoDiagnostic, Result};
6+
use rspack_error::{miette::IntoDiagnostic, Result, ToStringResultToRspackResultExt};
77
use rspack_hook::plugin_hook;
88
use rspack_loader_runner::State as LoaderState;
99

@@ -143,7 +143,17 @@ pub(crate) fn merge_loader_context(
143143
Some(content)
144144
}
145145
};
146-
let source_map = from.source_map.map(Into::into);
146+
let source_map = from
147+
.source_map
148+
.as_ref()
149+
.map(|s| {
150+
rspack_core::rspack_sources::SourceMap::from_json(
151+
// SAFETY: `sourceMap` is serialized by JavaScript from a JSON object. This is an invariant should be followed on the JavaScript side.
152+
unsafe { str::from_utf8_unchecked(s) },
153+
)
154+
})
155+
.transpose()
156+
.to_rspack_result()?;
147157
let additional_data = from.additional_data.take().map(|data| {
148158
let mut additional = AdditionalData::default();
149159
additional.insert(data);

crates/rspack_binding_api/src/source.rs

Lines changed: 4 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ use std::{hash::Hash, sync::Arc};
33
use napi_derive::napi;
44
use rspack_core::rspack_sources::{
55
BoxSource, CachedSource, ConcatSource, MapOptions, OriginalSource, RawBufferSource, RawSource,
6-
RawStringSource, ReplaceSource, Source, SourceExt, SourceMapSource, WithoutOriginalOptions,
6+
RawStringSource, ReplaceSource, Source, SourceExt, SourceMap, SourceMapSource,
7+
WithoutOriginalOptions,
78
};
89
use rspack_napi::napi::bindgen_prelude::*;
910

@@ -25,7 +26,7 @@ impl<'s> From<JsCompatSource<'s>> for BoxSource {
2526
match value.source {
2627
Either::A(string) => {
2728
if let Some(map) = value.map {
28-
match rspack_core::rspack_sources::SourceMap::from_slice(map.as_ref()).ok() {
29+
match SourceMap::from_slice(map.as_ref()).ok() {
2930
Some(source_map) => SourceMapSource::new(WithoutOriginalOptions {
3031
value: string,
3132
name: "inmemory://from js",
@@ -54,7 +55,7 @@ impl From<JsCompatSourceOwned> for BoxSource {
5455
match value.source {
5556
Either::A(string) => {
5657
if let Some(map) = value.map {
57-
match rspack_core::rspack_sources::SourceMap::from_slice(map.as_ref()).ok() {
58+
match SourceMap::from_slice(map.as_ref()).ok() {
5859
Some(source_map) => SourceMapSource::new(WithoutOriginalOptions {
5960
value: string,
6061
name: "inmemory://from js",
@@ -323,77 +324,3 @@ fn to_webpack_map(source: &dyn Source) -> Result<Option<String>> {
323324

324325
map.map(|m| m.to_json()).transpose().to_napi_result()
325326
}
326-
327-
#[napi(object)]
328-
pub struct SourceMap {
329-
pub file: Option<String>,
330-
pub sources: Option<Vec<Option<String>>>,
331-
pub source_root: Option<String>,
332-
pub sources_content: Option<Vec<Option<String>>>,
333-
pub names: Option<Vec<Option<String>>>,
334-
pub mappings: Option<String>,
335-
pub debug_id: Option<String>,
336-
}
337-
338-
impl From<&rspack_core::rspack_sources::SourceMap> for SourceMap {
339-
fn from(value: &rspack_core::rspack_sources::SourceMap) -> Self {
340-
SourceMap {
341-
file: value.file().map(|file| file.to_string()),
342-
sources: Some(
343-
value
344-
.sources()
345-
.iter()
346-
.map(|source| Some(source.to_string()))
347-
.collect::<Vec<_>>(),
348-
),
349-
source_root: value
350-
.source_root()
351-
.map(|source_root| source_root.to_string()),
352-
sources_content: Some(
353-
value
354-
.sources_content()
355-
.iter()
356-
.map(|content| Some(content.to_string()))
357-
.collect::<Vec<_>>(),
358-
),
359-
names: Some(
360-
value
361-
.names()
362-
.iter()
363-
.map(|name| Some(name.to_string()))
364-
.collect::<Vec<_>>(),
365-
),
366-
mappings: Some(value.mappings().to_string()),
367-
debug_id: value.get_debug_id().map(|id| id.to_string()),
368-
}
369-
}
370-
}
371-
372-
impl From<SourceMap> for rspack_core::rspack_sources::SourceMap {
373-
fn from(value: SourceMap) -> Self {
374-
let mappings = value.mappings.unwrap_or_default();
375-
let sources = value
376-
.sources
377-
.unwrap_or_default()
378-
.into_iter()
379-
.map(|source| source.unwrap_or_default())
380-
.collect::<Vec<_>>();
381-
let sources_content = value
382-
.sources_content
383-
.unwrap_or_default()
384-
.into_iter()
385-
.map(|source| source.unwrap_or_default())
386-
.collect::<Vec<_>>();
387-
let names = value
388-
.names
389-
.unwrap_or_default()
390-
.into_iter()
391-
.map(|name| name.unwrap_or_default())
392-
.collect::<Vec<_>>();
393-
let mut map =
394-
rspack_core::rspack_sources::SourceMap::new(mappings, sources, sources_content, names);
395-
map.set_source_root(value.source_root);
396-
map.set_debug_id(value.debug_id);
397-
map
398-
}
399-
}

packages/rspack/src/loader-runner/index.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,13 @@ import {
3838
isUseSourceMap
3939
} from "../config/adapterRuleUse";
4040
import { JavaScriptTracer } from "../trace";
41-
import { isNil, stringifyLoaderObject, toBuffer } from "../util";
41+
import {
42+
isNil,
43+
serializeObject,
44+
stringifyLoaderObject,
45+
toBuffer,
46+
toObject
47+
} from "../util";
4248
import { createHash } from "../util/createHash";
4349
import {
4450
absolutify,
@@ -214,6 +220,16 @@ export class LoaderObject {
214220
}
215221
}
216222

223+
class JsSourceMap {
224+
static __from_binding(map?: Buffer) {
225+
return isNil(map) ? undefined : toObject(map);
226+
}
227+
228+
static __to_binding(map?: object) {
229+
return serializeObject(map);
230+
}
231+
}
232+
217233
function dirname(path: string) {
218234
if (path === "/") return "/";
219235
const i = path.lastIndexOf("/");
@@ -1029,7 +1045,7 @@ export async function runLoaders(
10291045
if (hasArg) {
10301046
const [content, sourceMap, additionalData] = args;
10311047
context.content = isNil(content) ? null : toBuffer(content);
1032-
context.sourceMap = sourceMap || undefined;
1048+
context.sourceMap = serializeObject(sourceMap);
10331049
context.additionalData = additionalData || undefined;
10341050
break;
10351051
}
@@ -1039,7 +1055,7 @@ export async function runLoaders(
10391055
}
10401056
case JsLoaderState.Normal: {
10411057
let content = context.content;
1042-
let sourceMap = context.sourceMap;
1058+
let sourceMap = JsSourceMap.__from_binding(context.sourceMap);
10431059
let additionalData = context.additionalData;
10441060

10451061
while (loaderContext.loaderIndex >= 0) {
@@ -1069,7 +1085,7 @@ export async function runLoaders(
10691085
}
10701086

10711087
context.content = isNil(content) ? null : toBuffer(content);
1072-
context.sourceMap = sourceMap || undefined;
1088+
context.sourceMap = JsSourceMap.__to_binding(sourceMap);
10731089
context.additionalData = additionalData || undefined;
10741090
context.__internal__utf8Hint = typeof content === "string";
10751091

0 commit comments

Comments
 (0)