Skip to content

Commit ad7d27e

Browse files
authored
Turbopack hmr: preserve group factory consistency for compressed modules (#89976)
## Summary - update `installCompressedModuleFactories` to reuse an existing factory in a compressed ID group when filling missing IDs - keep `newModuleId` notifications aligned with actual IDs installed (`newModuleId(id)`) - add an execution regression test for the mixed-group case where one ID already has a factory and another is missing - regenerate affected runtime snapshots (runtime/debug-ids/workers/preset_env) Aims to fix cases like this being seen recently: ```sh Error: Module 8896 was instantiated because it was required from module 18500, but the module factory is not available. ``` ``` ## Test Plan - `cargo test -p turbopack-tests --test execution test_tests__execution__turbopack__runtime__factory_group_existing_factory__input__index_js -- --nocapture` - `cargo test -p turbopack-tests --test snapshot test_tests__snapshot__runtime__default_build_runtime__input__index_js -- --nocapture` - `cargo test -p turbopack-tests --test snapshot test_tests__snapshot__runtime__default_dev_runtime__input__index_js -- --nocapture` - `cargo test -p turbopack-tests --test snapshot test_tests__snapshot__debug_ids -- --nocapture` - `cargo test -p turbopack-tests --test snapshot test_tests__snapshot__swc_transforms__preset_env__input__index_js -- --nocapture` - `cargo test -p turbopack-tests --test snapshot test_tests__snapshot__workers__ -- --nocapture`
1 parent 6df0b7b commit ad7d27e

File tree

20 files changed

+297
-120
lines changed

20 files changed

+297
-120
lines changed

turbopack/crates/turbopack-ecmascript-runtime/js/src/shared/runtime/runtime-utils.ts

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,6 @@ function installCompressedModuleFactories(
551551
) {
552552
let i = offset
553553
while (i < chunkModules.length) {
554-
let moduleId = chunkModules[i] as ModuleId
555554
let end = i + 1
556555
// Find our factory function
557556
while (
@@ -565,19 +564,33 @@ function installCompressedModuleFactories(
565564
}
566565

567566
// Install the factory for each module ID that doesn't already have one.
568-
// This handles both the normal case and the case where some IDs in a group
569-
// may have been registered separately (e.g., from another chunk or HMR update).
567+
// When some IDs in this group already have a factory, reuse that existing
568+
// group factory for the missing IDs to keep all IDs in the group consistent.
569+
// Otherwise, install the factory from this chunk.
570570
const moduleFactoryFn = chunkModules[end] as Function
571+
let existingGroupFactory: Function | undefined = undefined
572+
for (let j = i; j < end; j++) {
573+
const id = chunkModules[j] as ModuleId
574+
const existingFactory = moduleFactories.get(id)
575+
if (existingFactory) {
576+
existingGroupFactory = existingFactory
577+
break
578+
}
579+
}
580+
const factoryToInstall = existingGroupFactory ?? moduleFactoryFn
581+
571582
let didInstallFactory = false
572583
for (let j = i; j < end; j++) {
573584
const id = chunkModules[j] as ModuleId
574585
if (!moduleFactories.has(id)) {
575586
if (!didInstallFactory) {
576-
applyModuleFactoryName(moduleFactoryFn)
577-
newModuleId?.(moduleId)
587+
if (factoryToInstall === moduleFactoryFn) {
588+
applyModuleFactoryName(moduleFactoryFn)
589+
}
578590
didInstallFactory = true
579591
}
580-
moduleFactories.set(id, moduleFactoryFn)
592+
moduleFactories.set(id, factoryToInstall)
593+
newModuleId?.(id)
581594
}
582595
}
583596
i = end + 1 // end is pointing at the last factory advance to the next id or the end of the array.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
const fs = require('fs')
2+
const os = require('os')
3+
const path = require('path')
4+
5+
function writeChunk(filePath, chunkModules) {
6+
fs.writeFileSync(filePath, `module.exports = ${chunkModules};\n`)
7+
}
8+
9+
it('reuses the existing group factory for missing IDs', () => {
10+
const repoRoot = path.resolve(process.cwd(), '../../../../../../../..')
11+
const runtimePath = path.join(
12+
repoRoot,
13+
'turbopack/crates/turbopack-tests/tests/snapshot/runtime/default_build_runtime/output/[turbopack]_runtime.js'
14+
)
15+
const createRuntime = eval('require')(runtimePath)
16+
17+
const suffix = `${Date.now()}-${Math.random().toString(16).slice(2)}`
18+
const moduleA = `module-a-${suffix}`
19+
const moduleB = `module-b-${suffix}`
20+
21+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tp-factory-group-'))
22+
try {
23+
const firstChunk = path.join(tempDir, 'first.js')
24+
const mixedChunk = path.join(tempDir, 'mixed.js')
25+
26+
writeChunk(
27+
firstChunk,
28+
`[
29+
${JSON.stringify(moduleA)},
30+
function (__turbopack_context__, module) {
31+
module.exports = "first-factory"
32+
}
33+
]`
34+
)
35+
36+
writeChunk(
37+
mixedChunk,
38+
`[
39+
${JSON.stringify(moduleA)},
40+
${JSON.stringify(moduleB)},
41+
function (__turbopack_context__, module) {
42+
module.exports = "second-factory"
43+
}
44+
]`
45+
)
46+
47+
const runtime = createRuntime('test-source')
48+
runtime.c(firstChunk)
49+
runtime.c(mixedChunk)
50+
51+
expect(runtime.m(moduleA).exports).toBe('first-factory')
52+
expect(runtime.m(moduleB).exports).toBe('first-factory')
53+
} finally {
54+
fs.rmSync(tempDir, { recursive: true, force: true })
55+
}
56+
})

turbopack/crates/turbopack-tests/tests/snapshot/debug-ids/browser/output/aaf3a_crates_turbopack-tests_tests_snapshot_debug-ids_browser_input_index_0151fefb.js.map

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/debug-ids/browser/output/ba425_crates_turbopack-tests_tests_snapshot_debug-ids_browser_input_index_0151fefb.js

Lines changed: 20 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/debug-ids/node/output/[turbopack]_runtime.js

Lines changed: 18 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/debug-ids/node/output/[turbopack]_runtime.js.map

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/runtime/default_build_runtime/output/[turbopack]_runtime.js

Lines changed: 18 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/runtime/default_build_runtime/output/[turbopack]_runtime.js.map

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/runtime/default_dev_runtime/output/5c1d0_turbopack-tests_tests_snapshot_runtime_default_dev_runtime_input_index_c0f7e0b0.js

Lines changed: 18 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

turbopack/crates/turbopack-tests/tests/snapshot/runtime/default_dev_runtime/output/780ce_turbopack-tests_tests_snapshot_runtime_default_dev_runtime_input_index_c0f7e0b0.js.map

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)