Skip to content

Commit cb4e384

Browse files
committed
Fix dynamic import() in classic scripts
1 parent 93d81a7 commit cb4e384

17 files changed

Lines changed: 561 additions & 126 deletions

File tree

β€Žcrates/html/src/package.rsβ€Ž

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{
99
dependencies::{is_func_iri_attr, parse_xml_stylesheet, serialize_xml_stylesheet},
1010
srcset::{parse_srcset, serialize_srcset},
1111
};
12-
use html5ever::{ExpandedName, expanded_name, local_name, namespace_url, ns};
12+
use html5ever::{ExpandedName, expanded_name, local_name, namespace_url, ns, tendril::StrTendril};
1313
use serde::Deserialize;
1414
use typed_arena::Arena;
1515

@@ -143,6 +143,37 @@ pub struct InlineBundle {
143143
pub module: bool,
144144
}
145145

146+
/// Escapes JavaScript for an HTML `<script>` element, whose content is raw text that a `</script`
147+
/// ends, and in which `<!--` can start a comment-like state that hides a following `</script>`.
148+
/// Replacing `</script` with `</\script` and `<!--` with `<\!--` keeps string and template literals
149+
/// equal, since `\s` and `\!` are identity escapes there, while `<\/script` could change a regular
150+
/// expression such as `0</script/`.
151+
fn escape_inline_script(code: &StrTendril) -> StrTendril {
152+
const CLOSE_TAG: &str = "</script";
153+
let mut escaped = String::with_capacity(code.len() + 8);
154+
let mut rest: &str = code;
155+
while let Some(index) = rest.find('<') {
156+
escaped.push_str(&rest[..index]);
157+
rest = &rest[index..];
158+
if rest.starts_with("<!--") {
159+
escaped.push_str("<\\!--");
160+
rest = &rest[4..];
161+
} else if rest
162+
.get(..CLOSE_TAG.len())
163+
.is_some_and(|tag| tag.eq_ignore_ascii_case(CLOSE_TAG))
164+
{
165+
escaped.push_str("</\\");
166+
escaped.push_str(&rest[2..CLOSE_TAG.len()]);
167+
rest = &rest[CLOSE_TAG.len()..];
168+
} else {
169+
escaped.push('<');
170+
rest = &rest[1..];
171+
}
172+
}
173+
escaped.push_str(rest);
174+
escaped.into()
175+
}
176+
146177
pub fn insert_bundle_references<'arena>(
147178
arena: &'arena Arena<Node<'arena>>,
148179
dom: &'arena Node<'arena>,
@@ -211,7 +242,12 @@ pub fn insert_bundle_references<'arena>(
211242
if let Some(key) = node.get_attribute(data_parcel_key.clone()) {
212243
if let Some(bundle) = inline_bundles.get(&SerializableTendril(key)) {
213244
node.remove_attribute(data_parcel_key.clone());
214-
node.set_text_content(arena, bundle.contents.0.clone());
245+
let contents = if name.ns == ns!(html) {
246+
escape_inline_script(&bundle.contents.0)
247+
} else {
248+
bundle.contents.0.clone()
249+
};
250+
node.set_text_content(arena, contents);
215251
if bundle.module {
216252
node.set_attribute(expanded_name!("", "type"), "module");
217253
}
@@ -392,6 +428,15 @@ mod tests {
392428
]
393429
}
394430

431+
#[test]
432+
fn inline_scripts_escape_closing_tags_and_comments() {
433+
let code: StrTendril = r#"a='</script>';b='</SCRIPT >';c='<!-- x';d=1<2;e='<scripts>'"#.into();
434+
assert_eq!(
435+
&*escape_inline_script(&code),
436+
r#"a='</\script>';b='</\SCRIPT >';c='<\!-- x';d=1<2;e='<scripts>'"#
437+
);
438+
}
439+
395440
#[test]
396441
fn positional_scripts_and_stylesheets_keep_order_and_deduplicate_per_page() {
397442
let result = package_html(PackageOptions {

β€Žcrates/js/src/packager/dependencies.rsβ€Ž

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,6 @@ pub(crate) fn bundle_dependencies(bundle_graph: &BundleGraph, bundle: &Bundle) -
3030
return bundle_graph.bundle_dependency_targets(bundle).collect();
3131
}
3232

33-
if bundle.target.source_type == SourceType::Script {
34-
// Classic scripts are emitted as is, without resolving dependencies.
35-
return Vec::new();
36-
}
37-
3833
let mut dependencies = Vec::new();
3934

4035
// A root's runtime maps the stable keys of everything its context loads to final names.

β€Žcrates/js/src/packager/mod.rsβ€Ž

Lines changed: 88 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ use std::{fmt::Write, sync::Arc};
22

33
use indexmap::{IndexMap, IndexSet};
44
use parcel_core::*;
5-
use parcel_js_swc_core::{Ast, async_module::wrap_async_module, tree_shake::tree_shake};
5+
use parcel_js_swc_core::{
6+
Ast,
7+
async_module::wrap_async_module,
8+
script::{link_script, minify_script},
9+
tree_shake::tree_shake,
10+
};
611
use swc_core::{
712
common::{DUMMY_SP, SyntaxContext},
813
ecma::ast::{
@@ -67,51 +72,27 @@ impl JsContent {
6772
const RUNTIME: &str = include_str!(concat!(env!("OUT_DIR"), "/runtime.min.js"));
6873
const DEV_RUNTIME: &str = include_str!("../dev-runtime.js");
6974

70-
if bundle.target.source_type == SourceType::Script {
71-
assert_eq!(bundle.assets.len(), 1);
72-
let asset = &bundle_graph
73-
.asset_graph
74-
.asset(bundle.main_entry_asset.unwrap());
75-
if let Some(content) = asset.content.downcast_ref::<JsContent>() {
76-
let should_optimize = bundle
77-
.target
78-
.flags
79-
.contains(EnvironmentFlags::SHOULD_OPTIMIZE);
80-
let mut ast = content.ast.clone();
81-
insert_node_replacements(&mut ast, content, asset, bundle, should_optimize);
82-
insert_node_replacement_helpers(
83-
&mut ast,
84-
content,
85-
bundle.target.output_format,
86-
should_optimize,
87-
);
88-
let (code, map) = content.to_code(
89-
&ast,
90-
bundle.target.source_map.is_some(),
91-
bundle_source_root(bundle, &options.project_root).as_deref(),
92-
false,
93-
)?;
94-
if let Some(map) = map {
95-
return Ok(Arc::new(ContentWithSourceMap::new(code, map.into_bytes())));
96-
}
97-
98-
return Ok(Arc::new(BufferContent::new(code)));
99-
}
100-
101-
return Ok(asset.content.clone());
102-
}
103-
10475
let should_build_source_map = bundle.target.source_map.is_some();
10576
let should_optimize = bundle
10677
.target
10778
.flags
10879
.contains(EnvironmentFlags::SHOULD_OPTIMIZE);
10980

81+
// A classic script's only asset runs unwrapped, so its top-level declarations are globals (see
82+
// `package_script`). What its dependencies resolve to goes in the module table before it.
83+
let script = if bundle.target.source_type == SourceType::Script {
84+
assert_eq!(bundle.assets.len(), 1);
85+
Some(bundle.assets[0])
86+
} else {
87+
None
88+
};
89+
11090
// Resolve dependencies before writing the bundle preamble. Some dependency
11191
// resolutions, such as synchronous bundles in ESM output, must be emitted
11292
// as top-level imports before the module table.
11393
let mut synthetic_assets = IndexSet::new();
11494
let mut asset_plans = Vec::with_capacity(bundle.assets.len());
95+
let mut script_dependencies = IndexMap::new();
11596
for asset_index in &bundle.assets {
11697
let asset = &bundle_graph.asset_graph.asset(*asset_index);
11798
let dependencies = asset_dependencies(
@@ -123,9 +104,32 @@ impl JsContent {
123104
get_inline_bundle_content,
124105
&options.project_root,
125106
)?;
126-
asset_plans.push((*asset_index, dependencies));
107+
if script == Some(*asset_index) {
108+
script_dependencies = dependencies;
109+
} else {
110+
asset_plans.push((*asset_index, dependencies));
111+
}
127112
}
128113

114+
let script = match script {
115+
Some(asset_index) => {
116+
let asset = bundle_graph.asset_graph.asset(asset_index);
117+
let Some(content) = asset.content.downcast_ref::<JsContent>() else {
118+
return Ok(asset.content.clone());
119+
};
120+
let script = package_script(content, asset, bundle, script_dependencies, options)?;
121+
// Without a module table, the script is the whole bundle.
122+
if synthetic_assets.is_empty() {
123+
return Ok(match script.1 {
124+
Some(map) => Arc::new(ContentWithSourceMap::new(script.0, map.into_bytes())),
125+
None => Arc::new(BufferContent::new(script.0)),
126+
});
127+
}
128+
Some(script)
129+
}
130+
None => None,
131+
};
132+
129133
// Async modules are entered through the runtime's module evaluator, and async entries evaluated
130134
// by it (see `tla-runtime.js`).
131135
if bundle
@@ -160,7 +164,10 @@ impl JsContent {
160164
}
161165
}
162166

163-
write_node_replacement_helpers(&mut printer, bundle_graph, bundle, options)?;
167+
// A script declares its own (see `package_script`).
168+
if script.is_none() {
169+
write_node_replacement_helpers(&mut printer, bundle_graph, bundle, options)?;
170+
}
164171
let externals = write_external_imports(&mut printer, bundle_graph, bundle)?;
165172
let sync_bundles =
166173
write_sync_bundle_imports(&mut printer, bundle_graph, bundle, &synthetic_assets)?;
@@ -263,10 +270,50 @@ impl JsContent {
263270
if is_global {
264271
printer.write_str("\n})();")?;
265272
}
273+
if let Some((code, map)) = script {
274+
printer.newline()?;
275+
printer.add_source_map(map)?;
276+
printer.write_str(std::str::from_utf8(&code).unwrap())?;
277+
}
266278
printer.into_content()
267279
}
268280
}
269281

282+
/// Generates a classic script, linking its dependencies through the global require of the module
283+
/// table before it, and minified without renaming its top-level declarations, which are globals.
284+
fn package_script(
285+
content: &JsContent,
286+
asset: &Asset,
287+
bundle: &Bundle,
288+
dependencies: IndexMap<String, Resolution<'_>>,
289+
options: &ParcelOptions,
290+
) -> Result<(Vec<u8>, Option<String>), DiagnosticList> {
291+
let should_optimize = bundle
292+
.target
293+
.flags
294+
.contains(EnvironmentFlags::SHOULD_OPTIMIZE);
295+
let mut ast = content.ast.clone();
296+
insert_node_replacements(&mut ast, content, asset, bundle, should_optimize);
297+
insert_node_replacement_helpers(
298+
&mut ast,
299+
content,
300+
bundle.target.output_format,
301+
should_optimize,
302+
);
303+
if !dependencies.is_empty() {
304+
link_script(&mut ast, dependencies, "parcelRequire".into());
305+
}
306+
if should_optimize {
307+
minify_script(&mut ast);
308+
}
309+
content.to_code(
310+
&ast,
311+
bundle.target.source_map.is_some(),
312+
bundle_source_root(bundle, &options.project_root).as_deref(),
313+
should_optimize,
314+
)
315+
}
316+
270317
fn sync_bundle_binding(bundle: &Bundle) -> String {
271318
format!("__parcelSyncBundle{:x}", bundle.id)
272319
}
@@ -837,7 +884,9 @@ fn write_runtime_globals(
837884
if let Some(entry) = rsc_server_entry {
838885
write!(printer, "'{}',", entry)?;
839886
}
840-
for entry in &bundle.entry_assets {
887+
// A classic script's entry runs after the module table rather than from it.
888+
let is_script = bundle.target.source_type == SourceType::Script;
889+
for entry in bundle.entry_assets.iter().filter(|_| !is_script) {
841890
write!(
842891
printer,
843892
"'{}',",
@@ -867,7 +916,7 @@ fn write_runtime_globals(
867916
if let Some(main) = bundle
868917
.main_entry_asset
869918
.as_ref()
870-
.filter(|_| !bundle.entry_assets.is_empty())
919+
.filter(|_| !bundle.entry_assets.is_empty() && !is_script)
871920
{
872921
printer.write_var(
873922
runtime_main_entry,
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!doctype html>
22
<script type="module">
33
import {a, b, c} from './script.js';
4-
output({a, b, c});
4+
output = {a, b, c};
55
</script>
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default 'async ' + globalVar;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<!doctype html>
2+
<html>
3+
<body>
4+
<script>
5+
var globalVar = 'global';
6+
import('./async.js').then(function (m) {
7+
output = m.default;
8+
});
9+
</script>
10+
<script>
11+
sideEffect(typeof globalVar);
12+
</script>
13+
</body>
14+
</html>
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
[
2+
{
3+
"description": "should load dynamic imports from inline classic scripts",
4+
"input": "index.html",
5+
"skipNodeModules": true,
6+
"bundles": [
7+
{"type": "html", "assets": ["index.html"]},
8+
{"type": "js", "assets": ["index.html"]},
9+
{"type": "js", "assets": ["index.html"]},
10+
{"type": "js", "assets": ["async.js"]}
11+
],
12+
"output": "async global",
13+
"sideEffects": ["string"]
14+
},
15+
{
16+
"description": "should load dynamic imports from inline classic scripts in production",
17+
"input": "index.html",
18+
"options": {"mode": "production"},
19+
"skipNodeModules": true,
20+
"bundles": [
21+
{"type": "html", "assets": ["index.html"]},
22+
{"type": "js", "assets": ["index.html"]},
23+
{"type": "js", "assets": ["index.html"]},
24+
{"type": "js", "assets": ["async.js"]}
25+
],
26+
"output": "async global",
27+
"sideEffects": ["string"]
28+
}
29+
]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default 'async ' + globalVar;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!doctype html>
2+
<html>
3+
<body>
4+
<script src="index.js"></script>
5+
<script>
6+
sideEffect(typeof globalVar);
7+
</script>
8+
</body>
9+
</html>
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
var globalVar = 'global';
2+
function load() {
3+
return import('./async.js');
4+
}
5+
load().then(function (m) {
6+
output = m.default;
7+
});

0 commit comments

Comments
Β (0)