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

fix: missing dependencies should be recoverable #5152

Merged
Merged
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
4 changes: 2 additions & 2 deletions crates/rspack_core/src/compiler/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl WorkerTask for FactorizeTask {
.with_diagnostics(diagnostics)
.with_file_dependencies(create_data.file_dependencies.drain())
.with_missing_dependencies(create_data.missing_dependencies.drain())
.with_context_dependencies(create_data.missing_dependencies.drain()),
.with_context_dependencies(create_data.context_dependencies.drain()),
)))
}
Err(mut e) => {
Expand All @@ -188,7 +188,7 @@ impl WorkerTask for FactorizeTask {
.with_diagnostics(diagnostics)
.with_file_dependencies(create_data.file_dependencies.drain())
.with_missing_dependencies(create_data.missing_dependencies.drain())
.with_context_dependencies(create_data.missing_dependencies.drain()),
.with_context_dependencies(create_data.context_dependencies.drain()),
)))
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/rspack_core/src/normal_module_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ impl NormalModuleFactory {
Ok(result) => result,
Err(err) => (Err(err), false),
};

match resource_data {
Ok(ResolveResult::Resource(resource)) => {
let uri = resource.full_path().display().to_string();
Expand Down Expand Up @@ -334,6 +335,8 @@ impl NormalModuleFactory {
));
}
Err(err) => {
data.add_file_dependencies(file_dependencies);
data.add_missing_dependencies(missing_dependencies);
// let mut file_dependencies = Default::default();
// let mut missing_dependencies = Default::default();
// let mut from_cache_result = from_cache;
Expand Down
14 changes: 7 additions & 7 deletions crates/rspack_core/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ impl Resource {

/// Main entry point for module resolution.
pub async fn resolve(
args: ResolveArgs<'_>,
mut args: ResolveArgs<'_>,
plugin_driver: &SharedPluginDriver,
) -> Result<ResolveResult, Error> {
let mut args = args;

let dep = ResolveOptionsWithDependencyType {
resolve_options: args.resolve_options.take(),
resolve_to_context: args.resolve_to_context,
Expand All @@ -71,14 +69,16 @@ pub async fn resolve(
let base_dir = args.context.clone();
let base_dir = base_dir.as_ref();

let mut context = Default::default();
let resolver = plugin_driver.resolver_factory.get(dep);
let result = resolver
.resolve(base_dir, args.specifier)
.resolve_with_context(base_dir, args.specifier, &mut context)
.map_err(|error| error.into_resolve_error(&args));

let (file_dependencies, missing_dependencies) = resolver.dependencies();
args.file_dependencies.extend(file_dependencies);
args.missing_dependencies.extend(missing_dependencies);
args.file_dependencies.extend(context.file_dependencies);
args
.missing_dependencies
.extend(context.missing_dependencies);

result
}
63 changes: 56 additions & 7 deletions crates/rspack_core/src/resolver/resolver_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,19 @@ use rspack_error::{
error, miette::miette, DiagnosticError, Error, ErrorExt, Severity, TraceableError,
};
use rspack_loader_runner::DescriptionData;
use rustc_hash::FxHashSet as HashSet;

use super::{ResolveResult, Resource};
use crate::{DependencyCategory, Resolve, ResolveArgs, ResolveOptionsWithDependencyType};

#[derive(Debug, Default, Clone)]
pub struct ResolveContext {
/// Files that was found on file system
pub file_dependencies: HashSet<PathBuf>,
/// Dependencies that was not found on file system
pub missing_dependencies: HashSet<PathBuf>,
}

/// Proxy to [nodejs_resolver::Error] or [oxc_resolver::ResolveError]
#[derive(Debug)]
pub enum ResolveInnerError {
Expand Down Expand Up @@ -145,13 +154,6 @@ impl Resolver {
}
}

/// Return `dependencies` from `enhanced-resolve`
///
/// Implementation is currently blank.
pub fn dependencies(&self) -> (Vec<PathBuf>, Vec<PathBuf>) {
(vec![], vec![])
}

/// Return the options from the resolver
pub fn options(&self) -> ResolveInnerOptions<'_> {
match self {
Expand Down Expand Up @@ -191,6 +193,53 @@ impl Resolver {
},
}
}

/// Resolve a specifier to a given path.
pub fn resolve_with_context(
&self,
path: &Path,
request: &str,
resolve_context: &mut ResolveContext,
) -> Result<ResolveResult, ResolveInnerError> {
match self {
Self::NodejsResolver(resolver, _) => resolver
.resolve(path, request)
.map(|result| match result {
nodejs_resolver::ResolveResult::Resource(r) => ResolveResult::Resource(Resource {
path: r.path,
query: r.query,
fragment: r.fragment,
description_data: r.description.map(|d| {
DescriptionData::new(d.dir().as_ref().to_path_buf(), Arc::clone(d.data().raw()))
}),
}),
nodejs_resolver::ResolveResult::Ignored => ResolveResult::Ignored,
})
.map_err(ResolveInnerError::NodejsResolver),
Self::OxcResolver(resolver) => {
let mut context = Default::default();
let result = resolver.resolve_with_context(path, request, &mut context);
resolve_context
.file_dependencies
.extend(context.file_dependencies);
resolve_context
.missing_dependencies
.extend(context.missing_dependencies);
match result {
Ok(r) => Ok(ResolveResult::Resource(Resource {
path: r.path().to_path_buf(),
query: r.query().map(ToString::to_string),
fragment: r.fragment().map(ToString::to_string),
description_data: r
.package_json()
.map(|d| DescriptionData::new(d.directory().to_path_buf(), Arc::clone(d.raw_json()))),
})),
Err(oxc_resolver::ResolveError::Ignored(_)) => Ok(ResolveResult::Ignored),
Err(error) => Err(ResolveInnerError::OxcResolver(error)),
}
}
}
}
}

impl ResolveInnerError {
Expand Down
40 changes: 40 additions & 0 deletions packages/playground/cases/file/missing-files/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { test, expect } from "@/fixtures";

test("missing files should be able to recover if being added back", async ({
page,
fileAction
}) => {
let overlay = page.frameLocator("#webpack-dev-server-client-overlay");
await expect(
overlay.getByText("Can't resolve './missing-file-1'")
).toBeVisible();
await expect(
overlay.getByText("Can't resolve './missing-file-2'")
).toBeVisible();

fileAction.updateFile(
"src/missing-file-1.js",
() => "export const a = 'missing-file-1'"
);

fileAction.updateFile(
"src/missing-file-2.js",
() => "export const b = 'missing-file-2'"
);

await expect(page.locator("#missing-file-1")).toHaveText("missing-file-1");
await expect(page.locator("#missing-file-2")).toHaveText("missing-file-2");

fileAction.deleteFile("src/missing-file-1.js");

await expect(
overlay.getByText("Can't resolve './missing-file-1'")
).toBeVisible();

fileAction.updateFile(
"src/missing-file-1.js",
() => "export const a = 'missing-file-1'"
);

await expect(page.locator("#missing-file-1")).toHaveText("missing-file-1");
});
25 changes: 25 additions & 0 deletions packages/playground/cases/file/missing-files/rspack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
module.exports = {
context: __dirname,
mode: "development",
entry: {
main: "./src/index.js"
},
devServer: {
hot: true
},
cache: false,
stats: "none",
infrastructureLogging: {
debug: false
},
builtins: {
html: [
{
template: "./src/index.html"
}
]
},
watchOptions: {
poll: 1000
}
};
15 changes: 15 additions & 0 deletions packages/playground/cases/file/missing-files/src/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8" />
<meta http-equiv="X-UA-Compatible" content="IE=edge" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Document</title>
</head>

<body>
<div id="root"></div>
</body>

</html>
8 changes: 8 additions & 0 deletions packages/playground/cases/file/missing-files/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @ts-nocheck
import { a } from "./missing-file-1";
const { b } = require("./missing-file-2");

document.getElementById("root").innerHTML = `
<span id="missing-file-1">${a}</span>
<span id="missing-file-2">${b}</span>
`;
11 changes: 8 additions & 3 deletions packages/rspack/tests/MultiCompiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,15 @@ describe("MultiCompiler", function () {

compiler.watch({}, err => {
if (err) {
done(err);
} else {
done();
compiler.close(() => {
done(err);
});
return;
}
compiler.close(err => {
if (err) return done(err);
done();
});
});
}, 20000);

Expand Down
2 changes: 1 addition & 1 deletion packages/rspack/tests/StatsTestCases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe("StatsTestCases", () => {

expect(statsJson).toMatchSnapshot();
let statsString = stats.toString(statsOptions);
expect(statsString.replace(/\n[ ]+│ /, "")).toMatchSnapshot();
expect(replace(statsString)).toMatchSnapshot();
});
});
});
Loading