Skip to content

Commit

Permalink
fix(es/module): Allow FileName::Anon from node resolver (#8686)
Browse files Browse the repository at this point in the history
**Related issue:**
 
 - Closes #8674
  • Loading branch information
kdy1 authored Mar 4, 2024
1 parent 7a5c7df commit 761365e
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 23 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ env:
# https://github.com/actions/setup-node/issues/899#issuecomment-1819151595
SKIP_YARN_COREPACK_CHECK: 1

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: "${{ github.event_name == 'pull_request' }}"

jobs:
cargo-fmt:
name: Cargo fmt
Expand Down
6 changes: 6 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8674/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"$schema": "http://json.schemastore.org/swcrc",
"jsc": {
"baseUrl": "."
}
}
2 changes: 2 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8674/input/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { foo } from "src/foo"
console.log(foo)
1 change: 1 addition & 0 deletions crates/swc/tests/fixture/issues-8xxx/8674/input/src/foo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { }
2 changes: 2 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8674/output/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { foo } from "./src/foo";
console.log(foo);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { };
32 changes: 32 additions & 0 deletions crates/swc/tests/projects.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{
env::current_dir,
fs::create_dir_all,
path::{Path, PathBuf},
};
Expand Down Expand Up @@ -1123,6 +1124,37 @@ fn issue_7513_2() {
);
}

#[test]
fn issue_8674_1() {
static INPUT: &str = "import { foo } from 'src/foo'";

let base_url = current_dir()
.unwrap()
.join("../../node-swc/tests/issue-8674")
.canonicalize()
.unwrap();

dbg!(&base_url);

let output = str_with_opt(
INPUT,
Options {
config: Config {
jsc: JscConfig {
base_url,
..Default::default()
},
..Default::default()
},
..Default::default()
},
)
.unwrap();
println!("{}", output);

assert_eq!(output.to_string(), "import { foo } from \"./src/foo\";\n");
}

#[testing::fixture("tests/minify/**/input.js")]
fn minify(input_js: PathBuf) {
let input_dir = input_js.parent().unwrap();
Expand Down
37 changes: 26 additions & 11 deletions crates/swc_ecma_loader/src/resolvers/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,27 @@ impl NodeModulesResolver {
Ok(None)
}

fn resolve_filename(&self, base: &FileName, target: &str) -> Result<FileName, Error> {
fn resolve_filename(&self, base: &FileName, module_specifier: &str) -> Result<FileName, Error> {
debug!(
"Resolving {} from {:#?} for {:#?}",
target, base, self.target_env
module_specifier, base, self.target_env
);

{
// Handle absolute path

let path = Path::new(module_specifier);

if let Ok(file) = self
.resolve_as_file(path)
.or_else(|_| self.resolve_as_directory(path, false))
{
if let Ok(file) = self.wrap(file) {
return Ok(file);
}
}
}

let base = match base {
FileName::Real(v) => v,
_ => bail!("node-resolver supports only files"),
Expand All @@ -437,10 +452,10 @@ impl NodeModulesResolver {
if let Some(pkg_base) = find_package_root(base) {
if let Some(item) = BROWSER_CACHE.get(&pkg_base) {
let value = item.value();
if value.module_ignores.contains(target) {
return Ok(FileName::Custom(target.into()));
if value.module_ignores.contains(module_specifier) {
return Ok(FileName::Custom(module_specifier.into()));
}
if let Some(rewrite) = value.module_rewrites.get(target) {
if let Some(rewrite) = value.module_rewrites.get(module_specifier) {
return self.wrap(Some(rewrite.to_path_buf()));
}
}
Expand All @@ -449,21 +464,21 @@ impl NodeModulesResolver {

// Handle builtin modules for nodejs
if let TargetEnv::Node = self.target_env {
if target.starts_with("node:") {
return Ok(FileName::Custom(target.into()));
if module_specifier.starts_with("node:") {
return Ok(FileName::Custom(module_specifier.into()));
}

if is_core_module(target) {
return Ok(FileName::Custom(format!("node:{}", target)));
if is_core_module(module_specifier) {
return Ok(FileName::Custom(format!("node:{}", module_specifier)));
}
}

// Aliases allow browser shims to be renamed so we can
// map `stream` to `stream-browserify` for example
let target = if let Some(alias) = self.alias.get(target) {
let target = if let Some(alias) = self.alias.get(module_specifier) {
&alias[..]
} else {
target
module_specifier
};

let target_path = Path::new(target);
Expand Down
6 changes: 1 addition & 5 deletions crates/swc_ecma_loader/src/resolvers/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,9 @@ where
}

let path = self.base_url.join(module_specifier);
#[cfg(windows)]
let path_string: String = path.to_string_lossy().replace("\\", "/");
#[cfg(not(windows))]
let path_string: String = path.to_string_lossy().to_string();

// https://www.typescriptlang.org/docs/handbook/modules/reference.html#baseurl
if let Ok(v) = self.invoke_inner_resolver(base, path_string.as_str()) {
if let Ok(v) = self.invoke_inner_resolver(base, &path.to_string_lossy()) {
return Ok(v);
}

Expand Down
4 changes: 3 additions & 1 deletion crates/swc_ecma_loader/tests/tsc_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,10 @@ struct TestResolver(AHashMap<String, String>);

impl Resolve for TestResolver {
fn resolve(&self, _: &FileName, src: &str) -> Result<Resolution, Error> {
let src = src.replace('\\', "/");

self.0
.get(src)
.get(&src)
.cloned()
.map(FileName::Custom)
.map(|v| Resolution {
Expand Down
15 changes: 9 additions & 6 deletions crates/swc_ecma_transforms_module/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,16 @@ where
v.parent()
.ok_or_else(|| anyhow!("failed to get parent of {:?}", v))?,
),
FileName::Anon => {
if cfg!(target_arch = "wasm32") {
panic!("Please specify `filename`")
} else {
Cow::Owned(current_dir().expect("failed to get current directory"))
FileName::Anon => match &self.config.base_dir {
Some(v) => Cow::Borrowed(&**v),
None => {
if cfg!(target_arch = "wasm32") {
panic!("Please specify `filename`")
} else {
Cow::Owned(current_dir().expect("failed to get current directory"))
}
}
}
},
_ => {
unreachable!(
"Node path provider does not support using `{:?}` as a base file name",
Expand Down
29 changes: 29 additions & 0 deletions node-swc/__tests__/transform/issue_8674_test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import path from "node:path";
import { fileURLToPath } from "node:url";
import swc from "../../..";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

it("should transpile import path correctly", async () => {
const baseUrl = path.resolve(__dirname, "../../tests/issue-8674");
console.log("baseUrl", baseUrl);
process.chdir(baseUrl);

const { code } = await swc.transform(
`
import { foo } from "src/foo"
console.log(foo)
`,
{
jsc: {
baseUrl,
},
}
);

expect(code).toMatchInlineSnapshot(`
"import { foo } from "./src/foo";
console.log(foo);
"
`);
});
1 change: 1 addition & 0 deletions node-swc/tests/issue-8674/src/bar.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'bar'
1 change: 1 addition & 0 deletions node-swc/tests/issue-8674/src/foo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 'foo'

0 comments on commit 761365e

Please sign in to comment.