Skip to content

Commit

Permalink
fix: should create an empty codegen result for module that has failur…
Browse files Browse the repository at this point in the history
…e in codegen (#7777)

fix: parse error in concatenatedModule should be emitted to diagnostics
  • Loading branch information
JSerFeng authored Sep 19, 2024
1 parent e5425f9 commit e32e58b
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 32 deletions.
52 changes: 35 additions & 17 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ use crate::{
unaffected_cache::UnaffectedModulesCache,
BoxDependency, BoxModule, CacheCount, CacheOptions, Chunk, ChunkByUkey, ChunkContentHash,
ChunkGraph, ChunkGroupByUkey, ChunkGroupUkey, ChunkKind, ChunkUkey, CodeGenerationJob,
CodeGenerationResults, CompilationLogger, CompilationLogging, CompilerOptions, DependencyId,
DependencyType, Entry, EntryData, EntryOptions, EntryRuntime, Entrypoint, ExecuteModuleId,
Filename, ImportVarMap, LocalFilenameFn, Logger, Module, ModuleFactory, ModuleGraph,
ModuleGraphPartial, ModuleIdentifier, PathData, ResolverFactory, RuntimeGlobals, RuntimeModule,
RuntimeSpecMap, SharedPluginDriver, SourceType, Stats,
CodeGenerationResult, CodeGenerationResults, CompilationLogger, CompilationLogging,
CompilerOptions, DependencyId, DependencyType, Entry, EntryData, EntryOptions, EntryRuntime,
Entrypoint, ExecuteModuleId, Filename, ImportVarMap, LocalFilenameFn, Logger, Module,
ModuleFactory, ModuleGraph, ModuleGraphPartial, ModuleIdentifier, PathData, ResolverFactory,
RuntimeGlobals, RuntimeModule, RuntimeSpecMap, SharedPluginDriver, SourceType, Stats,
};

pub type BuildDependency = (
Expand Down Expand Up @@ -870,19 +870,33 @@ impl Compilation {
.into_par_iter()
.map(|job| {
let module = job.module;
self
.old_cache
.code_generate_occasion
.use_cache(job, |module, runtime| {
let module = module_graph
.module_by_identifier(&module)
.expect("should have module");
module.code_generation(self, Some(runtime), None)
})
.map(|(res, runtimes, from_cache)| (module, res, runtimes, from_cache))
let runtimes = job.runtimes.clone();
(
module,
runtimes,
self
.old_cache
.code_generate_occasion
.use_cache(job, |module, runtime| {
let module = module_graph
.module_by_identifier(&module)
.expect("should have module");
module.code_generation(self, Some(runtime), None)
}),
)
})
.collect::<Result<Vec<_>>>()?;
for (module, codegen_res, runtimes, from_cache) in results {
.map(|(module, runtime, result)| match result {
Ok((result, runtime, from_cache)) => (module, result, runtime, from_cache, None),
Err(err) => (
module,
CodeGenerationResult::default(),
runtime,
false,
Some(err),
),
})
.collect::<Vec<_>>();
for (module, codegen_res, runtimes, from_cache, err) in results {
if let Some(counter) = cache_counter {
if from_cache {
counter.hit();
Expand All @@ -902,6 +916,10 @@ impl Compilation {
.add(module, runtime, codegen_res_id);
}
self.code_generated_modules.insert(module);

if let Some(err) = err {
self.push_diagnostic(Diagnostic::from(err).with_module_identifier(Some(module)));
}
}
Ok(())
}
Expand Down
28 changes: 13 additions & 15 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1730,21 +1730,19 @@ impl ConcatenatedModule {
Ok(res) => Program::Module(res),
Err(err) => {
let span: ErrorSpan = err.span().into();
self
.diagnostics
.lock()
.expect("should have diagnostics")
.append(&mut map_box_diagnostics_to_module_parse_diagnostics(vec![
rspack_error::TraceableError::from_source_file(
&fm,
span.start as usize,
span.end as usize,
"JavaScript parsing error".to_string(),
err.kind().msg().to_string(),
)
.with_kind(DiagnosticKind::JavaScript),
]));
return Ok(ModuleInfo::Concatenated(module_info));

// return empty error as we already push error to compilation.diagnostics
return Err(
rspack_error::TraceableError::from_source_file(
&fm,
span.start as usize,
span.end as usize,
"JavaScript parsing error:\n".to_string(),
err.kind().msg().to_string(),
)
.with_kind(DiagnosticKind::JavaScript)
.into(),
);
}
};
let mut ast = Ast::new(program, cm, Some(comments));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
console.log(DEFINE_VAR)
export default 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import v from './foo'

console.log(v)
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const rspack = require('@rspack/core')

/** @type {import('@rspack/core').Configuration} */
module.exports = {
plugins: [
new rspack.DefinePlugin({
DEFINE_VAR: '1 2 3'
})
],
optimization: {
concatenateModules: true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ERROR in ./index.js + 1 modules
× JavaScript parsing error:
│ : Expected ',', got 'numeric literal (2, 2)'
╭─[1:14]
1 │ console.log(1 2 3)
· ─
2 │ /* harmony default export */ const __WEBPACK_DEFAULT_EXPORT__ = (1);
╰────

2 comments on commit e32e58b

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rslib ❌ failure
rsbuild ❌ failure
examples ✅ success

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-09-19 0a89e43) Current Change
10000_development-mode + exec 2.22 s ± 58 ms 2.22 s ± 42 ms +0.29 %
10000_development-mode_hmr + exec 683 ms ± 3.5 ms 689 ms ± 30 ms +0.86 %
10000_production-mode + exec 2.85 s ± 30 ms 2.82 s ± 31 ms -1.01 %
arco-pro_development-mode + exec 1.81 s ± 72 ms 1.83 s ± 83 ms +0.84 %
arco-pro_development-mode_hmr + exec 433 ms ± 2.3 ms 433 ms ± 3.4 ms +0.03 %
arco-pro_production-mode + exec 3.27 s ± 41 ms 3.27 s ± 94 ms -0.16 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.29 s ± 97 ms 3.31 s ± 100 ms +0.64 %
threejs_development-mode_10x + exec 1.66 s ± 19 ms 1.67 s ± 19 ms +0.33 %
threejs_development-mode_10x_hmr + exec 791 ms ± 16 ms 789 ms ± 1.2 ms -0.29 %
threejs_production-mode_10x + exec 5.14 s ± 22 ms 5.16 s ± 29 ms +0.31 %

Please sign in to comment.