fix(core): reuse single module instance in jiti config loading on Windows#4306
Closed
claude[bot] wants to merge 1 commit into
Closed
fix(core): reuse single module instance in jiti config loading on Windows#4306claude[bot] wants to merge 1 commit into
claude[bot] wants to merge 1 commit into
Conversation
…dows On Windows, Forge loads TypeScript configs through jiti, whose internally-resolved module filenames are not case-canonicalized against Node's realpath-keyed require.cache. jiti's module-cache dedup therefore misses a dependency Node has already loaded and evaluates a second copy. When that dependency is webpack, the duplicate's Compilation.PROCESS_ASSETS_STAGE_* constants are undefined, so plugins tap processAssets at the wrong stage and silently drop index.html from the packaged app. Scope a fix to exactly the moment jiti is loaded: on win32 only, temporarily wrap Module.createRequire so the require jiti builds for itself gets a case-insensitive Proxy view over the same module cache, then restore it. Only jiti's own require view is affected; the global require.cache is never written or replaced. Off win32, jiti is loaded untouched. Fixes #3949
Contributor
Author
|
Closing in favor of #4311. The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Samuel Attard, Niklas Wenzel · Slack thread
Fixes #3949
Summarize your changes:
Before / after
Before, on Windows, a project with a TypeScript Forge config that uses the Webpack plugin would silently drop
index.htmlfrom the packaged app (a regression in v7.8.1). After this change,index.htmlis emitted again.How
Forge loads configs via jiti. On Windows (a case-insensitive filesystem), jiti's resolved module filenames aren't case-canonicalized against Node's realpath-keyed
require.cache, so jiti's internal module-cache dedup misses the copy Node already loaded and evaluates a second copy of a dependency. When that dependency is webpack, the duplicate copy'sCompilation.PROCESS_ASSETS_STAGE_*constants come backundefined, so plugins tapprocessAssetsat the wrong stage and the asset (e.g.index.html) never gets emitted.The fix wraps the
createRequirejiti captures at its own load time — win32 only, scoped to exactly therequire('jiti')call — so the require jiti builds for itself gets a case-insensitive view over the same module cache, and its dedup reuses the single already-loaded instance.Module.createRequireis restored in afinally.Properties of the fix:
requireview is affected; the globalrequire.cacheis never written or replaced. Off-win32, jiti is loaded completely untouched.Testing
Added focused unit coverage for the case-insensitive module-cache wrapper in
packages/api/core/spec/fast/util/forge-config.spec.ts(case-insensitiveget/hasresolution to the single loaded object, exact-key fast path, no-match behaviour, index invalidation on add/delete, and write-through to the underlying cache). The existingforge.config.ts/.cts/.mtsresolution tests continue to exercise the jiti load path.Note
This is a Windows-only regression, confirmed via the reporter's diagnostics. The dedup fix is verified on Linux by simulating the casing miss; final Windows confirmation from the reporter is in progress.