Skip to content

Commit

Permalink
feat: incremental for collecting dependencies diagnostics (#8127)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored and easy1090 committed Oct 17, 2024
1 parent 8fb89c6 commit 1af24a2
Show file tree
Hide file tree
Showing 24 changed files with 201 additions and 13 deletions.
1 change: 1 addition & 0 deletions crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1412,6 +1412,7 @@ export interface RawIncremental {
emitAssets: boolean
inferAsyncModules: boolean
providedExports: boolean
collectModulesDiagnostics: boolean
moduleHashes: boolean
moduleCodegen: boolean
moduleRuntimeRequirements: boolean
Expand Down
1 change: 1 addition & 0 deletions crates/rspack_binding_options/src/options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl TryFrom<RawOptions> for CompilerOptions {
emit_assets: value.emit_assets,
infer_async_modules: value.infer_async_modules,
provided_exports: value.provided_exports,
collect_modules_diagnostics: value.collect_modules_diagnostics,
module_hashes: value.module_hashes,
module_codegen: value.module_codegen,
module_runtime_requirements: value.module_runtime_requirements,
Expand Down
2 changes: 2 additions & 0 deletions crates/rspack_binding_options/src/options/raw_experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct RawIncremental {
pub emit_assets: bool,
pub infer_async_modules: bool,
pub provided_exports: bool,
pub collect_modules_diagnostics: bool,
pub module_hashes: bool,
pub module_codegen: bool,
pub module_runtime_requirements: bool,
Expand All @@ -29,6 +30,7 @@ impl From<RawIncremental> for Incremental {
emit_assets: value.emit_assets,
infer_async_modules: value.infer_async_modules,
provided_exports: value.provided_exports,
collect_modules_diagnostics: value.collect_modules_diagnostics,
module_hashes: value.module_hashes,
module_codegen: value.module_codegen,
module_runtime_requirements: value.module_runtime_requirements,
Expand Down
67 changes: 58 additions & 9 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ pub struct Compilation {
pub named_chunk_groups: HashMap<String, ChunkGroupUkey>,

pub async_modules: IdentifierSet,
pub modules_diagnostics: IdentifierMap<Vec<Diagnostic>>,
pub code_generation_results: CodeGenerationResults,
pub cgm_hash_results: CgmHashResults,
pub cgm_runtime_requirements_results: CgmRuntimeRequirementsResults,
Expand Down Expand Up @@ -266,6 +267,7 @@ impl Compilation {
named_chunk_groups: Default::default(),

async_modules: Default::default(),
modules_diagnostics: Default::default(),
code_generation_results: Default::default(),
cgm_hash_results: Default::default(),
cgm_runtime_requirements_results: Default::default(),
Expand Down Expand Up @@ -1071,8 +1073,17 @@ impl Compilation {
)],
)?;

if let Some(mutations) = &mut self.mutations
&& let Some(make_mutations) = self.make_artifact.take_mutations()
{
mutations.extend(make_mutations);
}

let incremental = self.options.incremental();
if incremental.infer_async_modules_enabled() || incremental.provided_exports_enabled() {
if incremental.infer_async_modules_enabled()
|| incremental.provided_exports_enabled()
|| incremental.collect_modules_diagnostics_enabled()
{
self
.unaffected_modules_cache
.compute_affected_modules_with_module_graph(self);
Expand All @@ -1087,7 +1098,7 @@ impl Compilation {
logger.time_end(start);
// Collect dependencies diagnostics at here to make sure:
// 1. after finish_modules: has provide exports info
// 2. before optimize dependencies: side effects free module hasn't been skipped (move_target)
// 2. before optimize dependencies: side effects free module hasn't been skipped
self.collect_dependencies_diagnostics();

// take make diagnostics
Expand All @@ -1110,16 +1121,54 @@ impl Compilation {

#[tracing::instrument(skip_all)]
fn collect_dependencies_diagnostics(&mut self) {
let incremental = self
.options
.incremental()
.collect_modules_diagnostics_enabled();
let modules = if incremental {
if let Some(mutations) = &self.mutations {
let revoked_modules = mutations.iter().filter_map(|mutation| match mutation {
Mutation::ModuleRevoke { module } => Some(*module),
_ => None,
});
for revoked_module in revoked_modules {
self.modules_diagnostics.remove(&revoked_module);
}
}
self
.unaffected_modules_cache
.get_affected_modules_with_module_graph()
.lock()
.expect("should lock")
.clone()
} else {
let module_graph = self.get_module_graph();
module_graph.modules().keys().copied().collect()
};
let module_graph = self.get_module_graph();
let diagnostics: Vec<_> = module_graph
.module_graph_modules()
let modules_diagnostics: IdentifierMap<Vec<Diagnostic>> = modules
.par_iter()
.flat_map(|(_, mgm)| &mgm.all_dependencies)
.filter_map(|dependency_id| module_graph.dependency_by_id(dependency_id))
.filter_map(|dependency| dependency.get_diagnostics(&module_graph))
.flat_map(|ds| ds)
.map(|module_identifier| {
let mgm = module_graph
.module_graph_module_by_identifier(module_identifier)
.expect("should have mgm");
let diagnostics = mgm
.all_dependencies
.iter()
.filter_map(|dependency_id| module_graph.dependency_by_id(dependency_id))
.filter_map(|dependency| dependency.get_diagnostics(&module_graph))
.flatten()
.collect::<Vec<_>>();
(*module_identifier, diagnostics)
})
.collect();
self.extend_diagnostics(diagnostics);
let all_modules_diagnostics = if incremental {
self.modules_diagnostics.extend(modules_diagnostics);
self.modules_diagnostics.clone()
} else {
modules_diagnostics
};
self.extend_diagnostics(all_modules_diagnostics.into_values().flatten());
}

#[instrument(name = "compilation:seal", skip_all)]
Expand Down
4 changes: 4 additions & 0 deletions crates/rspack_core/src/compiler/hmr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ impl Compiler {
if incremental.infer_async_modules_enabled() {
new_compilation.async_modules = std::mem::take(&mut self.compilation.async_modules);
}
if incremental.collect_modules_diagnostics_enabled() {
new_compilation.modules_diagnostics =
std::mem::take(&mut self.compilation.modules_diagnostics);
}
if incremental.module_hashes_enabled() {
new_compilation.cgm_hash_results = std::mem::take(&mut self.compilation.cgm_hash_results);
}
Expand Down
16 changes: 14 additions & 2 deletions crates/rspack_core/src/compiler/make/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use rustc_hash::FxHashSet as HashSet;

use self::{cutout::Cutout, repair::repair};
use crate::{
utils::FileCounter, BuildDependency, Compilation, DependencyId, ModuleGraph, ModuleGraphPartial,
ModuleIdentifier,
unaffected_cache::{Mutation, Mutations},
utils::FileCounter,
BuildDependency, Compilation, DependencyId, ModuleGraph, ModuleGraphPartial, ModuleIdentifier,
};

#[derive(Debug, Default)]
Expand All @@ -19,6 +20,7 @@ pub struct MakeArtifact {
// should be reset when rebuild
pub diagnostics: Vec<Diagnostic>,
pub has_module_graph_change: bool,
pub mutations: Option<Mutations>,

// data
pub built_modules: IdentifierSet,
Expand Down Expand Up @@ -48,6 +50,10 @@ impl MakeArtifact {
&mut self.module_graph_partial
}

pub fn take_mutations(&mut self) -> Option<Mutations> {
std::mem::take(&mut self.mutations)
}

pub fn take_diagnostics(&mut self) -> Vec<Diagnostic> {
std::mem::take(&mut self.diagnostics)
}
Expand Down Expand Up @@ -75,6 +81,11 @@ impl MakeArtifact {
.build_dependencies
.remove_batch_file(&build_info.build_dependencies);
}
if let Some(mutations) = &mut self.mutations {
mutations.add(Mutation::ModuleRevoke {
module: *module_identifier,
});
}
module_graph.revoke_module(module_identifier)
}

Expand Down Expand Up @@ -133,6 +144,7 @@ pub fn make_module_graph(
// reset temporary data
artifact.built_modules = Default::default();
artifact.diagnostics = Default::default();
artifact.mutations = compilation.mutations.is_some().then(Default::default);
artifact.has_module_graph_change = false;

artifact = update_module_graph(compilation, artifact, params)?;
Expand Down
7 changes: 7 additions & 0 deletions crates/rspack_core/src/compiler/module_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl ModuleExecutor {
params.push(MakeParam::ForceBuildModules(modules));
}
make_artifact.diagnostics = Default::default();
make_artifact.mutations = compilation.mutations.is_some().then(Default::default);
make_artifact.has_module_graph_change = false;

make_artifact = update_module_graph(compilation, make_artifact, params).unwrap_or_default();
Expand Down Expand Up @@ -134,6 +135,12 @@ impl ModuleExecutor {
let diagnostics = self.make_artifact.take_diagnostics();
compilation.extend_diagnostics(diagnostics);

if let Some(mutations) = &mut compilation.mutations
&& let Some(make_mutations) = self.make_artifact.take_mutations()
{
mutations.extend(make_mutations);
}

let built_modules = self.make_artifact.take_built_modules();
for id in built_modules {
compilation.built_modules.insert(id);
Expand Down
5 changes: 5 additions & 0 deletions crates/rspack_core/src/options/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub enum Incremental {
emit_assets: bool,
infer_async_modules: bool,
provided_exports: bool,
collect_modules_diagnostics: bool,
module_hashes: bool,
module_codegen: bool,
module_runtime_requirements: bool,
Expand Down Expand Up @@ -45,6 +46,10 @@ impl Incremental {
matches!(self, Incremental::Enabled { provided_exports, .. } if *provided_exports)
}

pub fn collect_modules_diagnostics_enabled(&self) -> bool {
matches!(self, Incremental::Enabled { collect_modules_diagnostics, .. } if *collect_modules_diagnostics)
}

pub fn module_hashes_enabled(&self) -> bool {
matches!(self, Incremental::Enabled { module_hashes, .. } if *module_hashes)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/unaffected_cache/mutations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ pub struct Mutations {

#[derive(Debug)]
pub enum Mutation {
ModuleRevoke { module: ModuleIdentifier },
ModuleSetAsync { module: ModuleIdentifier },
PlaceholderForExtendable,
}

impl Mutations {
Expand Down
1 change: 1 addition & 0 deletions packages/rspack-test-tools/src/case/new-incremental.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const watchCreator = new BasicCaseCreator({
emitAssets: true,
inferAsyncModules: true,
providedExports: true,
collectModulesDiagnostics: true,
moduleHashes: true,
moduleCodegen: true,
moduleRuntimeRequirements: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export class HotNewIncrementalProcessor<
emitAssets: true,
inferAsyncModules: true,
providedExports: true,
collectModulesDiagnostics: true,
moduleHashes: true,
moduleCodegen: true,
moduleRuntimeRequirements: true
Expand Down
65 changes: 64 additions & 1 deletion packages/rspack-test-tools/src/processor/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { merge } from "webpack-merge";

import { ECompilerEvent } from "../compiler";
import { readConfigFile } from "../helper";
import checkArrayExpectation from "../helper/legacy/checkArrayExpectation";
import copyDiff from "../helper/legacy/copyDiff";
import type {
ECompilerType,
Expand Down Expand Up @@ -82,7 +83,69 @@ export class WatchProcessor<
}

async check(env: ITestEnv, context: ITestContext) {
await super.check(env, context);
const testConfig = context.getTestConfig();
if (testConfig.noTest) return;

const errors: Array<{ message: string; stack?: string }> = (
context.getError(this._options.name) || []
).map(e => ({
message: e.message,
stack: e.stack
}));
const warnings: Array<{ message: string; stack?: string }> = [];
const compiler = this.getCompiler(context);
const stats = compiler.getStats();
if (stats) {
fs.writeFileSync(
path.join(context.getDist(), "stats.txt"),
stats.toString({
preset: "verbose",
colors: false
}),
"utf-8"
);
const jsonStats = stats.toJson({
errorDetails: true
});
fs.writeFileSync(
path.join(context.getDist(), "stats.json"),
JSON.stringify(jsonStats, null, 2),
"utf-8"
);
if (jsonStats.errors) {
errors.push(...jsonStats.errors);
}
if (jsonStats.warnings) {
warnings.push(...jsonStats.warnings);
}
}
await new Promise<void>((resolve, reject) => {
checkArrayExpectation(
path.join(context.getSource(), this._watchOptions.stepName),
{ errors },
"error",
"Error",
reject
);
resolve();
});

await new Promise<void>((resolve, reject) => {
checkArrayExpectation(
path.join(context.getSource(), this._watchOptions.stepName),
{ warnings },
"warning",
"Warning",
reject
);
resolve();
});

// clear error if checked
if (fs.existsSync(context.getSource("errors.js"))) {
context.clearError(this._options.name);
}

// check hash
fs.renameSync(
path.join(context.getDist(), "stats.txt"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Object {
"css": undefined,
"futureDefaults": false,
"incremental": Object {
"collectModulesDiagnostics": false,
"emitAssets": true,
"inferAsyncModules": false,
"make": true,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { notExist } from "./value"

export const value = notExist;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./value"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { value } from "./good";

it("should have correct value", () => {
expect(value).toBe(1);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const value = 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { value } from "./bad";

it("should have correct value", () => {
expect(value).toBe(undefined);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = [
/export 'notExist' \(imported as 'notExist'\) was not found in '\.\/value'/,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { value } from "./good";

it("should have correct value", () => {
expect(value).toBe(1);
});
Loading

0 comments on commit 1af24a2

Please sign in to comment.