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

feat: incremental for collecting dependencies diagnostics #8127

Merged
merged 5 commits into from
Oct 17, 2024
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
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
Loading