Skip to content

Commit 1d79b02

Browse files
shulaodashulaoda
authored
fix: use sentinel id for browser: false ignored modules (#9192)
closes #9107
1 parent 31d0403 commit 1d79b02

8 files changed

Lines changed: 80 additions & 28 deletions

File tree

‎crates/rolldown/src/utils/load_source.rs‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,20 @@ pub async fn load_source<Fs: FileSystem + 'static>(
2121
is_read_from_disk: &mut bool,
2222
module_idx: ModuleIdx,
2323
) -> anyhow::Result<(StrOrBytes, ModuleType)> {
24-
let (maybe_source, mut maybe_module_type) = match plugin_driver
25-
.load(&HookLoadArgs { id: &resolved_id.id, module_idx, asserted_module_type })
26-
.await?
27-
{
28-
Some(load_hook_output) => {
29-
sourcemap_chain.extend(load_hook_output.map.map(SourcemapChainElement::Load));
30-
if let Some(v) = load_hook_output.side_effects {
31-
*side_effects = Some(v);
32-
}
33-
34-
(Some(load_hook_output.code.to_string()), load_hook_output.module_type)
35-
}
36-
_ => {
37-
if resolved_id.ignored {
38-
(Some(String::new()), Some(ModuleType::Empty))
39-
} else {
40-
(None, None)
41-
}
42-
}
24+
let (maybe_source, mut maybe_module_type) = if resolved_id.id.is_empty_module() {
25+
(Some(String::new()), Some(ModuleType::Empty))
26+
} else {
27+
plugin_driver
28+
.load(&HookLoadArgs { id: &resolved_id.id, module_idx, asserted_module_type })
29+
.await?
30+
.map(|load_hook_output| {
31+
sourcemap_chain.extend(load_hook_output.map.map(SourcemapChainElement::Load));
32+
if let Some(v) = load_hook_output.side_effects {
33+
*side_effects = Some(v);
34+
}
35+
(Some(load_hook_output.code.to_string()), load_hook_output.module_type)
36+
})
37+
.unwrap_or_default()
4338
};
4439

4540
// If we're given a specific module type to use and the load hook did not provide a

‎crates/rolldown_common/src/types/module_id.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ use sugar_path::SugarPath;
88

99
use super::stable_module_id::StableModuleId;
1010

11+
const EMPTY_MODULE_PREFIX: &str = "\0rolldown/empty.js?";
12+
1113
/// `ModuleId` is the unique string identifier for each module.
1214
/// - It will be used to identify the module in the whole bundle.
1315
/// - Users could stored the `ModuleId` to track the module in different stages/hooks.
@@ -28,6 +30,22 @@ impl ModuleId {
2830
Self { inner }
2931
}
3032

33+
/// Construct the sentinel id used for `browser: false` ignored modules,
34+
/// concatenated with the original resolved path so each ignored module
35+
/// stays distinguishable while sharing the empty-module load behavior.
36+
pub fn new_empty(original: &str) -> Self {
37+
Self::new(format!("{EMPTY_MODULE_PREFIX}{original}"))
38+
}
39+
40+
pub fn is_empty_module(&self) -> bool {
41+
self.inner.starts_with(EMPTY_MODULE_PREFIX)
42+
}
43+
44+
/// For an id created via `new_empty`, returns the original id portion.
45+
pub fn strip_empty_prefix(&self) -> Option<&str> {
46+
self.inner.strip_prefix(EMPTY_MODULE_PREFIX)
47+
}
48+
3149
pub fn as_str(&self) -> &str {
3250
&self.inner
3351
}

‎crates/rolldown_common/src/types/resolved_id.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ impl From<bool> for ResolvedExternal {
3737
#[derive(Debug, Default, Clone)]
3838
pub struct ResolvedId {
3939
pub id: ModuleId,
40-
// https://github.com/defunctzombie/package-browser-field-spec/blob/8c4869f6a5cb0de26d208de804ad0a62473f5a03/README.md?plain=1#L62-L77
41-
pub ignored: bool,
4240
pub module_def_format: ModuleDefFormat,
4341
pub external: ResolvedExternal,
4442
// If the js side is return object, the relative id is finally id, else it will be converted to an absolute id
@@ -54,7 +52,6 @@ impl ResolvedId {
5452
pub fn make_dummy() -> Self {
5553
Self {
5654
id: ModuleId::default(),
57-
ignored: false,
5855
module_def_format: ModuleDefFormat::Unknown,
5956
external: false.into(),
6057
normalize_external_id: None,
@@ -72,14 +69,16 @@ impl ResolvedId {
7269
return format!("<{}>", self.id.as_str());
7370
}
7471

75-
let stable = stabilize_id(&self.id, cwd.as_ref());
76-
if self.ignored { format!("(ignored) {stable}") } else { stable }
72+
if let Some(original) = self.id.strip_empty_prefix() {
73+
return format!("(ignored) {}", stabilize_id(original, cwd.as_ref()));
74+
}
75+
76+
stabilize_id(&self.id, cwd.as_ref())
7777
}
7878

7979
pub fn new_external_without_side_effects(id: ArcStr) -> Self {
8080
Self {
8181
id: ModuleId::new(id),
82-
ignored: false,
8382
module_def_format: ModuleDefFormat::Unknown,
8483
external: true.into(),
8584
normalize_external_id: None,

‎crates/rolldown_plugin/src/utils/resolve_id_with_plugins.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,7 @@ fn resolve_id<Fs: FileSystem>(
161161
..Default::default()
162162
}),
163163
ResolveError::Ignored(p) => Ok(ResolvedId {
164-
//(hyf0) TODO: This `p` doesn't seem to contains `query` or `fragment` of the input. We need to make sure this is ok
165-
id: ModuleId::new(p.to_str().expect("Should be valid utf8")),
166-
ignored: true,
164+
id: ModuleId::new_empty(p.to_str().expect("Should be valid utf8")),
167165
..Default::default()
168166
}),
169167
_ => Err(err),

‎packages/rolldown/tests/fixtures/plugin/context/resolve-browser-false/_config.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { defineTest } from 'rolldown-tests';
2+
import { expect, vi } from 'vitest';
3+
4+
const onResolved = vi.fn();
5+
6+
export default defineTest({
7+
config: {
8+
input: './main.js',
9+
platform: 'browser',
10+
plugins: [
11+
{
12+
name: 'probe',
13+
async resolveId(source, importer, options) {
14+
if (source !== 'buffer') {
15+
return;
16+
}
17+
const resolved = await this.resolve(source, importer, {
18+
...options,
19+
skipSelf: true,
20+
});
21+
onResolved(resolved);
22+
return resolved;
23+
},
24+
},
25+
],
26+
},
27+
afterTest() {
28+
expect(onResolved).toHaveBeenCalledTimes(1);
29+
expect(onResolved).toHaveBeenCalledWith(
30+
expect.objectContaining({ id: expect.stringContaining('\0rolldown/empty.js?') }),
31+
);
32+
},
33+
});

‎packages/rolldown/tests/fixtures/plugin/context/resolve-browser-false/demo/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import 'buffer';

‎packages/rolldown/tests/fixtures/plugin/context/resolve-browser-false/demo/package.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "demo",
3+
"type": "module",
4+
"browser": {
5+
"buffer": false
6+
}
7+
}

‎packages/rolldown/tests/fixtures/plugin/context/resolve-browser-false/main.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
import './demo/index.js';

0 commit comments

Comments
 (0)