Skip to content

Commit b6286aa

Browse files
shulaodashulaoda
authored
Merge branch 'main' into 04-22-fix_preserve_ignored_flag_through_plugin_this.resolve_round-trip
2 parents 2b6eb41 + 31d0403 commit b6286aa

19 files changed

Lines changed: 360 additions & 49 deletions

File tree

‎crates/rolldown/src/bundler/impl_bundler_hmr.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::sync::{Arc, atomic::AtomicU32};
99

1010
impl Bundler {
1111
#[cfg(feature = "experimental")]
12+
#[tracing::instrument(level = "debug", skip_all)]
1213
pub async fn compute_hmr_update_for_file_changes(
1314
&mut self,
1415
changed_file_paths: &FxIndexMap<String, WatcherChangeKind>,

‎crates/rolldown/src/stages/generate_stage/chunk_optimizer.rs‎

Lines changed: 24 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -209,57 +209,35 @@ impl ChunkOptimizationGraph {
209209
source_chunk_idx: ChunkIdx,
210210
target_chunk_idx: ChunkIdx,
211211
) -> bool {
212-
let source_has_deps = !self.chunks[source_chunk_idx].dependencies.is_empty();
213-
214-
if source_has_deps {
215-
// When source has dependencies, only BFS from source's deps.
216-
// Including target's deps causes false positives because the simulation
217-
// finds that target can trivially "reach itself" through any transitive
218-
// dependency that also depends on source.
219-
let mut queue: VecDeque<ChunkIdx> =
220-
self.chunks[source_chunk_idx].dependencies.iter().copied().collect();
221-
let mut visited = FxHashSet::default();
212+
// Start BFS from the combined deps of source and target.
213+
let mut queue: VecDeque<ChunkIdx> = self.chunks[source_chunk_idx]
214+
.dependencies
215+
.iter()
216+
.chain(self.chunks[target_chunk_idx].dependencies.iter())
217+
.copied()
218+
.collect();
219+
let mut visited = FxHashSet::default();
222220

223-
while let Some(chunk_idx) = queue.pop_front() {
224-
if chunk_idx == target_chunk_idx {
225-
return true;
226-
}
227-
if !visited.insert(chunk_idx) {
228-
continue;
229-
}
230-
for &dep in &self.chunks[chunk_idx].dependencies {
231-
if !visited.contains(&dep) {
232-
queue.push_back(dep);
233-
}
234-
}
221+
while let Some(chunk_idx) = queue.pop_front() {
222+
if chunk_idx == target_chunk_idx {
223+
return true;
235224
}
236-
false
237-
} else {
238-
// When source has no dependencies (e.g., runtime chunk), we must check
239-
// from target's deps with post-merge simulation to detect cycles like
240-
// target -> chunk_A -> source(=target after merge).
241-
let mut queue: VecDeque<ChunkIdx> =
242-
self.chunks[target_chunk_idx].dependencies.iter().copied().collect();
243-
let mut visited = FxHashSet::default();
244-
245-
while let Some(chunk_idx) = queue.pop_front() {
246-
if chunk_idx == target_chunk_idx {
247-
return true;
248-
}
249-
if !visited.insert(chunk_idx) {
250-
continue;
251-
}
252-
for &dep in &self.chunks[chunk_idx].dependencies {
253-
if !visited.contains(&dep) {
254-
queue.push_back(dep);
255-
}
256-
}
257-
if self.chunks[chunk_idx].dependencies.contains(&source_chunk_idx) {
258-
queue.push_back(target_chunk_idx);
225+
if !visited.insert(chunk_idx) {
226+
continue;
227+
}
228+
for &dep in &self.chunks[chunk_idx].dependencies {
229+
if !visited.contains(&dep) {
230+
queue.push_back(dep);
259231
}
260232
}
261-
false
233+
// Any chunk that depends on source will depend on target after the merge.
234+
// Simulate this by also queuing target when we encounter such a chunk.
235+
if self.chunks[chunk_idx].dependencies.contains(&source_chunk_idx) {
236+
queue.push_back(target_chunk_idx);
237+
}
262238
}
239+
240+
false
263241
}
264242

265243
/// Returns true if `from` can transitively reach `to` through the dependency graph.

‎crates/rolldown/tests/rolldown/issues/7449/rolldown.config.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { defineConfig } from 'rolldown';
2+
3+
export default defineConfig({
4+
input: import.meta.input,
5+
});

‎crates/rolldown/tests/rolldown/issues/9225/_config.json‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"config": {
3+
"input": [
4+
{
5+
"name": "main",
6+
"import": "main.js"
7+
}
8+
],
9+
"platform": "node",
10+
"preserveEntrySignatures": "false",
11+
"manualCodeSplitting": {
12+
"includeDependenciesRecursively": false,
13+
"groups": [
14+
{
15+
"name": "api",
16+
"test": "api\\.js"
17+
}
18+
]
19+
}
20+
}
21+
}

‎crates/rolldown/tests/rolldown/issues/9225/_test.mjs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import assert from 'node:assert';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
4+
5+
const distDir = path.join(import.meta.dirname, 'dist');
6+
const jsFiles = fs
7+
.readdirSync(distDir)
8+
.filter((file) => file.endsWith('.js'))
9+
.sort();
10+
11+
const graph = Object.fromEntries(
12+
jsFiles.map((file) => {
13+
const code = fs.readFileSync(path.join(distDir, file), 'utf8');
14+
const imports = [
15+
...code.matchAll(/(?:import|export)\s+(?:[^"']*?\s+from\s+)?["']\.\/([^"']+)["']/g),
16+
].map((match) => match[1]);
17+
return [file, imports];
18+
}),
19+
);
20+
21+
function findCycle() {
22+
const visited = new Set();
23+
const inStack = new Set();
24+
25+
function dfs(file, pathSoFar) {
26+
if (inStack.has(file)) {
27+
return pathSoFar.slice(pathSoFar.indexOf(file)).concat(file);
28+
}
29+
if (visited.has(file)) {
30+
return null;
31+
}
32+
visited.add(file);
33+
inStack.add(file);
34+
for (const dep of graph[file] ?? []) {
35+
const cycle = dfs(dep, pathSoFar.concat(file));
36+
if (cycle) {
37+
return cycle;
38+
}
39+
}
40+
inStack.delete(file);
41+
return null;
42+
}
43+
44+
for (const file of Object.keys(graph)) {
45+
const cycle = dfs(file, []);
46+
if (cycle) {
47+
return cycle;
48+
}
49+
}
50+
return null;
51+
}
52+
53+
assert.strictEqual(
54+
findCycle(),
55+
null,
56+
`Output chunks must not have circular static imports: ${JSON.stringify(graph)}`,
57+
);
58+
59+
await import('./dist/main.js');
60+
assert.strictEqual(globalThis.__rolldown_issue_7449_value, 300000);
61+
assert.strictEqual(globalThis.__rolldown_issue_7449_side, 1);
62+
63+
await Promise.all(globalThis.__rolldown_issue_7449_imports);
64+
assert.strictEqual(globalThis.__rolldown_issue_7449_side, 1);

‎crates/rolldown/tests/rolldown/issues/9225/api.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { getEnvInt } from './env.js';
2+
3+
export const api = getEnvInt('REQUEST_TIMEOUT_MS');

‎crates/rolldown/tests/rolldown/issues/9225/artifacts.snap‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
---
2+
source: crates/rolldown_testing/src/integration_test.rs
3+
---
4+
# Assets
5+
6+
## api.js
7+
8+
```js
9+
import { t as getEnvInt } from "./env.js";
10+
//#region api.js
11+
const api = getEnvInt("REQUEST_TIMEOUT_MS");
12+
//#endregion
13+
export { api as t };
14+
15+
```
16+
17+
## dep-user.js
18+
19+
```js
20+
import "./dep.js";
21+
//#region dep-user.js
22+
const depUser = "dep";
23+
//#endregion
24+
export { depUser };
25+
26+
```
27+
28+
## dep.js
29+
30+
```js
31+
//#region side.js
32+
globalThis.__rolldown_issue_7449_side = (globalThis.__rolldown_issue_7449_side || 0) + 1;
33+
//#endregion
34+
export {};
35+
36+
```
37+
38+
## env-user.js
39+
40+
```js
41+
import { n as getEnvString } from "./env.js";
42+
//#region env-user.js
43+
const envUser = getEnvString("SECONDARY");
44+
//#endregion
45+
export { envUser };
46+
47+
```
48+
49+
## env.js
50+
51+
```js
52+
import "./dep.js";
53+
//#region env.js
54+
const state = { config: {
55+
REQUEST_TIMEOUT_MS: "300000",
56+
marker: "dep"
57+
} };
58+
function getEnvString(key) {
59+
return state.config?.[key];
60+
}
61+
function getEnvInt(key) {
62+
return Number(getEnvString(key) || 3e5);
63+
}
64+
//#endregion
65+
export { getEnvString as n, getEnvInt as t };
66+
67+
```
68+
69+
## lazy.js
70+
71+
```js
72+
import { t as api } from "./api.js";
73+
//#region lazy.js
74+
const lazy = api;
75+
//#endregion
76+
export { lazy };
77+
78+
```
79+
80+
## main.js
81+
82+
```js
83+
import { n as getEnvString } from "./env.js";
84+
import { t as api } from "./api.js";
85+
//#region main.js
86+
globalThis.__rolldown_issue_7449_imports = [
87+
import("./lazy.js"),
88+
import("./env-user.js"),
89+
import("./dep-user.js")
90+
];
91+
globalThis.__rolldown_issue_7449_value = api + Number(getEnvString("MISSING") || 0);
92+
//#endregion
93+
export {};
94+
95+
```

‎crates/rolldown/tests/rolldown/issues/9225/dep-user.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { marker } from './dep.js';
2+
3+
export const depUser = marker;

‎crates/rolldown/tests/rolldown/issues/9225/dep.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import './side.js';
2+
3+
export const marker = 'dep';

‎crates/rolldown/tests/rolldown/issues/9225/env-user.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { getEnvString } from './env.js';
2+
3+
export const envUser = getEnvString('SECONDARY');

0 commit comments

Comments
 (0)