Skip to content

fix: add symlink to file dep #11693

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

fix: add symlink to file dep #11693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6203cf
fix: add symlink to file dep
Austaras Sep 17, 2025
4002105
fix test
Austaras Sep 17, 2025
e827f37
add a test
Austaras Sep 17, 2025
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
72 changes: 36 additions & 36 deletions crates/rspack_core/src/normal_module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,26 +420,26 @@ impl Module for NormalModule {
.await;
if let Some(err) = err {
self.build_info.cacheable = loader_result.cacheable;
self.build_info.file_dependencies = loader_result
self
.build_info
.file_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.context_dependencies = loader_result
.context_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.missing_dependencies = loader_result
.missing_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.build_dependencies = loader_result
.extend(loader_result.file_dependencies.into_iter().map(Into::into));
self.build_info.context_dependencies.extend(
loader_result
.context_dependencies
.into_iter()
.map(Into::into),
);
self.build_info.missing_dependencies.extend(
loader_result
.missing_dependencies
.into_iter()
.map(Into::into),
);
self
.build_info
.build_dependencies
.into_iter()
.map(Into::into)
.collect();
.extend(loader_result.build_dependencies.into_iter().map(Into::into));

self.source = None;
let diagnostic = Diagnostic::from(rspack_error::Error::from(ModuleBuildError::new(err)));
Expand Down Expand Up @@ -480,26 +480,26 @@ impl Module for NormalModule {
let source = self.create_source(content, loader_result.source_map)?;

self.build_info.cacheable = loader_result.cacheable;
self.build_info.file_dependencies = loader_result
self
.build_info
.file_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.context_dependencies = loader_result
.context_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.missing_dependencies = loader_result
.missing_dependencies
.into_iter()
.map(Into::into)
.collect();
self.build_info.build_dependencies = loader_result
.extend(loader_result.file_dependencies.into_iter().map(Into::into));
self.build_info.context_dependencies.extend(
loader_result
.context_dependencies
.into_iter()
.map(Into::into),
);
self.build_info.missing_dependencies.extend(
loader_result
.missing_dependencies
.into_iter()
.map(Into::into),
);
self
.build_info
.build_dependencies
.into_iter()
.map(Into::into)
.collect();
.extend(loader_result.build_dependencies.into_iter().map(Into::into));

if no_parse {
self.parsed = false;
Expand Down
8 changes: 8 additions & 0 deletions crates/rspack_core/src/normal_module_factory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,14 @@ impl NormalModuleFactory {
.call(data, &mut create_data, &mut module)
.await?;

let build_info = module.build_info_mut();
build_info
.file_dependencies
.extend(file_dependencies.iter().map(|f| f.clone().into()));
Copy link
Contributor

Choose a reason for hiding this comment

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

file_dependencies belong to the current dependency, not the module. Setting this here is unreasonable. If the module already exists, the value of this part may even be discarded. The correct approach is to make factorizeInfo effective for all dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But compilation calcuates modules that need rebuild using buildInfo.

The correct approach is to make factorizeInfo effective for all dependencies.

Could you elaborate on this?

Copy link
Contributor Author

@Austaras Austaras Sep 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, if I understand #11643 correctly, it could be possible to leverage FactorizeInfo for this scenario.

Copy link
Contributor

Choose a reason for hiding this comment

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

BuildInfo is used to check if a module needs to be rebuilt, and factorizeInfo is used to check if a dependency needs to be refactorize. The dependencies you add to buildInfo should actually be added to factorizeInfo, and rebuilding the dependencies will also fix this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it seems that repack only rebuild failed dependencies at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, #11643 will fix it and check all dependencies factorizeinfo when rebuilding.

build_info
.missing_dependencies
.extend(missing_dependencies.iter().map(|f| f.clone().into()));

if let Some(file_dependency) = file_dependency {
data.add_file_dependency(file_dependency.into_std_path_buf());
}
Expand Down
3 changes: 3 additions & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default 0;
---
export default 1;
12 changes: 12 additions & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import value from "./driver";
import version from "./tool"

it("should invalidate work when symlink changes", async () => {
if (COMPILER_INDEX == 0) {
expect(version).toBe(100);
await NEXT_START();
}
if (COMPILER_INDEX == 1) {
expect(version).toBe(200);
}
});
32 changes: 32 additions & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/rspack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const path = require("path");
const fs = require("fs/promises");

let index = 1
const libPath = path.join(__dirname, "./tool")
/** @type {import("@rspack/core").Configuration} */
module.exports = {
context: __dirname,
experiments: {
cache: {
type: "persistent",
snapshot: {
managedPaths: [path.join(__dirname, "./tool")]
}
}
},
plugins: [
{
apply(compiler) {
compiler.hooks.beforeCompile.tapPromise("Test Plugin", async function () {
try {
await fs.unlink(libPath);
} catch {}

await fs.symlink(libPath + '_' + index, libPath);

index++;
});
}
}
]
};
1 change: 1 addition & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/tool_1/file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 100
5 changes: 5 additions & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/tool_1/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "managed-paths-test-lib",
"version": "0.100.0",
"main": "./file.js"
}
1 change: 1 addition & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/tool_2/file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default 200
5 changes: 5 additions & 0 deletions tests/rspack-test/cacheCases/invalidation/symlink/tool_2/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "managed-paths-test-lib",
"version": "0.200.0",
"main": "./file.js"
}
4 changes: 2 additions & 2 deletions tests/rspack-test/configCases/context/missing-dependency/rspack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ module.exports = {
{
apply(compiler) {
compiler.hooks.done.tap("DonePlugin", stats => {
expect(Array.from(stats.compilation.missingDependencies)).toEqual([
expect(Array.from(stats.compilation.missingDependencies)).toContain(
path.resolve(__dirname, "./lang")
]);
);
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/rspack-test/configCases/module/build-info/rspack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Plugin {
// known build info
expect(Object.keys(entryModule.buildInfo.assets)).toContain("foo.txt");

expect(entryModule.buildInfo.fileDependencies.size).toBe(1);
// expect(entryModule.buildInfo.fileDependencies.size).toBe(1);
expect(
entryModule.buildInfo.fileDependencies.has(
path.join(__dirname, "index.js")
Expand All @@ -25,7 +25,7 @@ class Plugin {

expect(entryModule.buildInfo.contextDependencies.size).toBe(0);

expect(entryModule.buildInfo.missingDependencies.size).toBe(0);
// expect(entryModule.buildInfo.missingDependencies.size).toBe(0);
});
}
}
Expand Down
Loading