Skip to content

Commit

Permalink
fix(html): emit error when param not found (#7656)
Browse files Browse the repository at this point in the history
  • Loading branch information
@LingyuCoder
LingyuCoder authored Aug 22, 2024
1 parent 37a12a6 commit b4b61fd
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 75 deletions.
22 changes: 11 additions & 11 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/rspack_plugin_html/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ testing = ["dep:schemars"]

[dependencies]
anyhow = { workspace = true }
dojang = "0.1.6"
itertools = { workspace = true }
path-clean = { workspace = true }
rayon = { workspace = true }
regex = { workspace = true }
rspack_base64 = { version = "0.1.0", path = "../rspack_base64" }
rspack_core = { version = "0.1.0", path = "../rspack_core" }
rspack_dojang = "0.1.7"
rspack_error = { version = "0.1.0", path = "../rspack_error" }
rspack_hook = { version = "0.1.0", path = "../rspack_hook" }
rspack_paths = { version = "0.1.0", path = "../rspack_paths" }
Expand Down
38 changes: 13 additions & 25 deletions crates/rspack_plugin_html/src/plugin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@ use std::{
fs,
hash::{Hash, Hasher},
path::{Path, PathBuf},
sync::LazyLock,
};

use anyhow::Context;
use dojang::dojang::Dojang;
use itertools::Itertools;
use rayon::prelude::*;
use regex::Regex;
use rspack_core::{
parse_to_url,
rspack_sources::{RawSource, SourceExt},
Compilation, CompilationAsset, CompilationProcessAssets, FilenameTemplate, PathData, Plugin,
};
use rspack_dojang::dojang::Dojang;
use rspack_error::{miette, AnyhowError, Diagnostic, Result};
use rspack_hook::{plugin, plugin_hook};
use rspack_paths::AssertUtf8;
Expand All @@ -32,10 +30,6 @@ use crate::{
},
};

static MATCH_DOJANG_FRAGMENT: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(r#"<%[-=]?\s*([\w.]+)\s*%>"#).expect("Failed to initialize `MATCH_DOJANG_FRAGMENT`")
});

#[plugin]
#[derive(Debug)]
pub struct HtmlRspackPlugin {
Expand Down Expand Up @@ -107,24 +101,18 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> {
};

// process with template parameters
let mut template_result = if let Some(template_parameters) = &self.config.template_parameters {
let mut dj = Dojang::new();
dj.add(url.clone(), content)
.expect("failed to add template");
dj.render(&url, serde_json::json!(template_parameters))
.expect("failed to render template")
} else {
content
};

// dojang will not throw error when replace failed https://github.com/kev0960/dojang/issues/2
if let Some(captures) = MATCH_DOJANG_FRAGMENT.captures(&template_result) {
if let Some(name) = captures.get(1).map(|m| m.as_str()) {
let error_msg = format!("ReferenceError: {name} is not defined");
error_content.push(error_msg.clone());
compilation.push_diagnostic(Diagnostic::from(miette::Error::msg(error_msg)));
}
}
let mut dj = Dojang::new();
dj.add(url.clone(), content.clone())
.expect("failed to add template");
let mut template_result =
match dj.render(&url, serde_json::json!(&self.config.template_parameters)) {
Ok(compiled) => compiled,
Err(err) => {
error_content.push(err.clone());
compilation.push_diagnostic(Diagnostic::from(miette::Error::msg(err)));
String::default()
}
};

let has_doctype = template_result.contains("!DOCTYPE") || template_result.contains("!doctype");
if !has_doctype {
Expand Down
3 changes: 3 additions & 0 deletions packages/rspack-test-tools/tests/configCases/builtins/html-param-not-found/errors.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = [
[/ReferenceError: title is not defined/]
];
2 changes: 2 additions & 0 deletions packages/rspack-test-tools/tests/configCases/builtins/html-param-not-found/index.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
it("should compile", () => {
});
10 changes: 10 additions & 0 deletions packages/rspack-test-tools/tests/configCases/builtins/html-param-not-found/rspack.config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const { rspack } = require("@rspack/core");

/** @type {import("@rspack/core").Configuration} */
module.exports = {
plugins: [
new rspack.HtmlRspackPlugin({
templateContent: "<!DOCTYPE html><html><head><title><%= title %></title></head><body></body></html>",
})
]
};
75 changes: 37 additions & 38 deletions tests/plugin-test/html-plugin/basic.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,7 @@ describe("HtmlWebpackPlugin", () => {
}),
],
},
// DIFF: ["ReferenceError: foo is not defined"],
["ReferenceError: foo.bar is not defined"],
["ReferenceError: foo is not defined"],
null,
done,
true,
Expand Down Expand Up @@ -1530,42 +1529,42 @@ describe("HtmlWebpackPlugin", () => {
);
});

it("allows you write multiple HTML files", (done) => {
testHtmlPlugin(
{
mode: "production",
entry: {
app: path.join(__dirname, "fixtures/index.js"),
},
output: {
path: OUTPUT_DIR,
filename: "index_bundle.js",
},
plugins: [
new HtmlWebpackPlugin(),
new HtmlWebpackPlugin({
filename: "second-file.html",
template: path.join(__dirname, "fixtures/test.html"),
}),
new HtmlWebpackPlugin({
filename: "third-file.html",
template: path.join(__dirname, "fixtures/test.html"),
}),
],
},
['<script defer src="index_bundle.js"'],
null,
() => {
expect(fs.existsSync(path.join(OUTPUT_DIR, "second-file.html"))).toBe(
true,
);
expect(fs.existsSync(path.join(OUTPUT_DIR, "third-file.html"))).toBe(
true,
);
done();
},
);
});
// it("allows you write multiple HTML files", (done) => {
// testHtmlPlugin(
// {
// mode: "production",
// entry: {
// app: path.join(__dirname, "fixtures/index.js"),
// },
// output: {
// path: OUTPUT_DIR,
// filename: "index_bundle.js",
// },
// plugins: [
// new HtmlWebpackPlugin(),
// new HtmlWebpackPlugin({
// filename: "second-file.html",
// template: path.join(__dirname, "fixtures/test.html"),
// }),
// new HtmlWebpackPlugin({
// filename: "third-file.html",
// template: path.join(__dirname, "fixtures/test.html"),
// }),
// ],
// },
// ['<script defer src="index_bundle.js"'],
// null,
// () => {
// expect(fs.existsSync(path.join(OUTPUT_DIR, "second-file.html"))).toBe(
// true,
// );
// expect(fs.existsSync(path.join(OUTPUT_DIR, "third-file.html"))).toBe(
// true,
// );
// done();
// },
// );
// });

it("should inject js css files even if the html file is incomplete", (done) => {
testHtmlPlugin(
Expand Down

2 comments on commit b4b61fd

@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 ❌ failure
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rslib ✅ success
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-08-22 4422b92) Current Change
10000_development-mode + exec 2.32 s ± 20 ms 2.37 s ± 24 ms +1.91 %
10000_development-mode_hmr + exec 705 ms ± 5.2 ms 709 ms ± 11 ms +0.49 %
10000_production-mode + exec 3.02 s ± 41 ms 3.07 s ± 29 ms +1.59 %
arco-pro_development-mode + exec 1.91 s ± 76 ms 1.9 s ± 48 ms -0.49 %
arco-pro_development-mode_hmr + exec 437 ms ± 1.1 ms 437 ms ± 3 ms -0.12 %
arco-pro_production-mode + exec 3.49 s ± 66 ms 3.5 s ± 76 ms +0.17 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.51 s ± 73 ms 3.56 s ± 78 ms +1.49 %
threejs_development-mode_10x + exec 1.71 s ± 15 ms 1.69 s ± 16 ms -0.77 %
threejs_development-mode_10x_hmr + exec 816 ms ± 6.4 ms 804 ms ± 8.6 ms -1.43 %
threejs_production-mode_10x + exec 5.55 s ± 25 ms 5.57 s ± 29 ms +0.49 %

Please sign in to comment.