From f96227cbc077820cc0e08d489976880ae33e47e0 Mon Sep 17 00:00:00 2001 From: Cong-Cong Pan Date: Wed, 30 Oct 2024 19:35:14 +0800 Subject: [PATCH] fix: sourceMapFilename is relative (#8269) --- Cargo.lock | 1 - crates/rspack_plugin_devtool/Cargo.toml | 1 - .../src/source_map_dev_tool_plugin.rs | 24 +++++++---- crates/rspack_util/src/path.rs | 41 +++++++++++++++++-- .../output/source-map-filename/index.js | 12 ++++-- .../source-map-filename/rspack.config.js | 2 +- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d90968ffc98e..ccaa29eed763 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4071,7 +4071,6 @@ dependencies = [ "derivative", "futures", "itertools 0.13.0", - "pathdiff", "rayon", "regex", "rspack_base64", diff --git a/crates/rspack_plugin_devtool/Cargo.toml b/crates/rspack_plugin_devtool/Cargo.toml index 757c1b1582a2..8d76cb0576ed 100644 --- a/crates/rspack_plugin_devtool/Cargo.toml +++ b/crates/rspack_plugin_devtool/Cargo.toml @@ -14,7 +14,6 @@ dashmap = { workspace = true } derivative = { workspace = true } futures = { workspace = true } itertools = { workspace = true } -pathdiff = { workspace = true } rayon = { workspace = true } regex = { workspace = true } rspack_base64 = { version = "0.1.0", path = "../rspack_base64" } diff --git a/crates/rspack_plugin_devtool/src/source_map_dev_tool_plugin.rs b/crates/rspack_plugin_devtool/src/source_map_dev_tool_plugin.rs index 2c79bf9cc444..0629a8f49a43 100644 --- a/crates/rspack_plugin_devtool/src/source_map_dev_tool_plugin.rs +++ b/crates/rspack_plugin_devtool/src/source_map_dev_tool_plugin.rs @@ -1,3 +1,4 @@ +use std::path::{Component, PathBuf}; use std::sync::LazyLock; use std::{borrow::Cow, path::Path}; @@ -5,7 +6,6 @@ use cow_utils::CowUtils; use derivative::Derivative; use futures::future::{join_all, BoxFuture}; use itertools::Itertools; -use pathdiff::diff_paths; use rayon::prelude::*; use regex::Regex; use rspack_core::{ @@ -416,13 +416,23 @@ impl SourceMapDevToolPlugin { if let Some(current_source_mapping_url_comment) = current_source_mapping_url_comment { let source_map_url = if let Some(public_path) = &self.public_path { - Cow::Owned(format!("{public_path}{source_map_filename}")) - } else if let Some(dirname) = Path::new(filename.as_ref()).parent() - && let Some(relative) = diff_paths(&source_map_filename, dirname) - { - Cow::Owned(relative.to_string_lossy().to_string()) + format!("{public_path}{source_map_filename}") } else { - Cow::Borrowed(source_map_filename.as_str()) + let mut file_path = PathBuf::new(); + file_path.push(Component::RootDir); + file_path.extend(Path::new(filename.as_ref()).components()); + + let mut source_map_path = PathBuf::new(); + source_map_path.push(Component::RootDir); + source_map_path.extend(Path::new(&source_map_filename).components()); + + relative( + #[allow(clippy::unwrap_used)] + file_path.parent().unwrap(), + &source_map_path, + ) + .to_string_lossy() + .to_string() }; let data = data.url(&source_map_url); let current_source_mapping_url_comment = match ¤t_source_mapping_url_comment { diff --git a/crates/rspack_util/src/path.rs b/crates/rspack_util/src/path.rs index 5fc1e3a3ee25..a714589e85e4 100644 --- a/crates/rspack_util/src/path.rs +++ b/crates/rspack_util/src/path.rs @@ -1,12 +1,43 @@ -use std::path::{Path, PathBuf}; +use std::path::{Component, Components, Path, PathBuf}; + +fn normalize(parts: Components, allow_above_root: bool) -> Vec { + let mut res = vec![]; + for p in parts { + match &p { + Component::CurDir => (), + Component::ParentDir => { + if !matches!( + res.last(), + Some(Component::ParentDir) | Some(Component::RootDir) + ) { + res.pop(); + } else if allow_above_root { + res.push(p); + } + } + _ => res.push(p), + } + } + res +} pub fn relative(from: &Path, to: &Path) -> PathBuf { if from == to { return PathBuf::new(); } - let mut from_iter = from.components(); - let mut to_iter = to.components(); + let is_from_absolute = matches!(from.components().next(), Some(Component::RootDir)); + let is_to_absolute = matches!(to.components().next(), Some(Component::RootDir)); + + // At this point the path should be resolved to a full absolute path, but + // handle relative paths to be safe + + // Normalize the path + let from = normalize(from.components(), !is_from_absolute); + let to = normalize(to.components(), !is_to_absolute); + + let mut from_iter = from.iter(); + let mut to_iter = to.iter(); let mut common_parts = 0; let from_remain = loop { @@ -24,7 +55,7 @@ pub fn relative(from: &Path, to: &Path) -> PathBuf { result.push(".."); } - let to_iter = to.components().skip(common_parts); + let to_iter = to.into_iter().skip(common_parts); for part in to_iter { result.push(part.as_os_str()); } @@ -59,6 +90,8 @@ mod test { ("/baz-quux", "/baz", "../baz"), ("/baz", "/baz-quux", "../baz-quux"), ("/page1/page2/foo", "/", "../../.."), + // Fix https://github.com/web-infra-dev/rspack/issues/8219 + ("/", "/../maps/main.js.map", "maps/main.js.map"), ]; for (from, to, expected) in test_cases { diff --git a/packages/rspack-test-tools/tests/configCases/output/source-map-filename/index.js b/packages/rspack-test-tools/tests/configCases/output/source-map-filename/index.js index 8fb96034d6a7..83b3b18f66ec 100644 --- a/packages/rspack-test-tools/tests/configCases/output/source-map-filename/index.js +++ b/packages/rspack-test-tools/tests/configCases/output/source-map-filename/index.js @@ -1,8 +1,12 @@ -import fs from "fs"; +import fs from "fs/promises"; import path from "path"; + it("source-map-filename/name should same", async function () { import("./two"); - expect( - fs.readdirSync(path.resolve(__dirname, "")).includes("main.js.map") - ).toBe(true); + + expect(async () => await fs.stat(path.resolve(__dirname, "../maps/main.js.map"))).not.toThrow(); + + const outputCode = await fs.readFile(__filename, 'utf-8'); + const sourceMapPath = outputCode.match(/\/\/# sourceMappingURL=(.*)/)?.[1]; + expect(sourceMapPath).toBe(path.normalize("maps/main.js.map")); }); diff --git a/packages/rspack-test-tools/tests/configCases/output/source-map-filename/rspack.config.js b/packages/rspack-test-tools/tests/configCases/output/source-map-filename/rspack.config.js index 60d1717471d0..edfac9fa51f9 100644 --- a/packages/rspack-test-tools/tests/configCases/output/source-map-filename/rspack.config.js +++ b/packages/rspack-test-tools/tests/configCases/output/source-map-filename/rspack.config.js @@ -7,6 +7,6 @@ module.exports = { target: "node", output: { filename: "[name].js", - sourceMapFilename: "[name].js.map" + sourceMapFilename: "../maps/[name].js.map" } };