Add vite-plugin-rescript for ReScript JSX source mapping#4
Conversation
Adds a new @treelocator/vite-plugin-rescript package and a rescript
adapter shim so Alt+click on a ReScript-React component points at the
original .res file instead of the compiled .res.js intermediate.
The plugin runs over .res.js files emitted with `"jsx": { "preserve":
true }`, looks each JSX opening tag up via the .res.js.map source map,
and injects __source attributes (fileName/lineNumber/columnNumber)
that flow through React's classic JSX runtime into fiber._debugSource.
It also appends `make.displayName = "ModuleName"` after the top-level
`let make = ...` so the fiber reports the file's module name instead
of the literal `make`.
Includes 17 unit tests for the plugin (filtering, source-map
remapping, displayName injection, missing-map handling, dev-mode
gating, and HMR cache invalidation), a Playwright ancestry spec for
the new demo, and a vite-rescript demo app with pre-compiled
.res.js + .res.js.map fixtures so the e2e flow runs without a
rescript binary in CI.
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider adding a
buildEnd/closeBundle/closeWatcherhook that iteratesconsumerCacheand callsdestroy()on eachSourceMapConsumerto ensure there are no lingering native resources in long-lived dev/CI processes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a `buildEnd`/`closeBundle`/`closeWatcher` hook that iterates `consumerCache` and calls `destroy()` on each `SourceMapConsumer` to ensure there are no lingering native resources in long-lived dev/CI processes.
## Individual Comments
### Comment 1
<location path="packages/vite-plugin-rescript/src/index.ts" line_range="115-118" />
<code_context>
+ if (!entry) {
+ if (!warnedNoMap.has(filePath)) {
+ warnedNoMap.add(filePath);
+ this.warn(
+ `vite-plugin-rescript: no source map (.map) found alongside ${filePath}. ` +
+ `Make sure rescript.json sets "jsx": { "version": 4, "preserve": true } and that source maps are emitted.`
+ );
</code_context>
<issue_to_address>
**suggestion:** Malformed source maps are reported as missing, which can mislead debugging.
In `getConsumer`, JSON parse failures for `.map` files also return `null`, so this warning currently conflates missing maps with malformed ones. That makes it harder to distinguish configuration issues from corrupted/invalid maps. Please either emit a distinct warning for parse failures (e.g., via a separate `warnedBadMap` set) or adjust this message to mention that the source map may be missing *or malformed*.
```suggestion
this.warn(
`vite-plugin-rescript: no valid source map (.map) found alongside ${filePath}. ` +
`The source map may be missing or malformed. Make sure rescript.json sets "jsx": { "version": 4, "preserve": true } and that source maps are emitted.`
);
```
</issue_to_address>
### Comment 2
<location path="packages/vite-plugin-rescript/src/index.ts" line_range="48-50" />
<code_context>
+export function vitePluginRescript(
+ options: VitePluginRescriptOptions = {}
+): Plugin {
+ const consumerCache = new Map<string, ConsumerEntry>();
+ const warnedNoMap = new Set<string>();
+ let isDev = false;
+
+ async function getConsumer(jsFilePath: string): Promise<ConsumerEntry | null> {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Cached SourceMapConsumers are never destroyed on plugin shutdown.
`consumerCache` is only cleaned up via `invalidateJs`/`invalidateRes`/HMR, so if the dev server stops or a build finishes without those being called, `SourceMapConsumer.destroy()` is never invoked. This can leak native resources in long dev sessions or large projects. Please add a `buildEnd`/`closeBundle` (and possibly `closeWatcher`) hook to iterate `consumerCache.values()`, call `destroy()` on each, and clear the map.
Suggested implementation:
```typescript
export function vitePluginRescript(
options: VitePluginRescriptOptions = {}
): Plugin {
const consumerCache = new Map<string, ConsumerEntry>();
const warnedNoMap = new Set<string>();
let isDev = false;
function cleanupConsumers() {
for (const entry of consumerCache.values()) {
try {
entry.consumer.destroy();
} catch {
// ignore errors during cleanup
}
}
consumerCache.clear();
}
async function getConsumer(jsFilePath: string): Promise<ConsumerEntry | null> {
```
To fully implement the cleanup:
1. In the object that `vitePluginRescript` returns (the `Plugin`), add lifecycle hooks that call `cleanupConsumers()`:
- `buildEnd() { cleanupConsumers(); }`
- `closeBundle() { cleanupConsumers(); }`
- Optionally `closeWatcher() { cleanupConsumers(); }` if the plugin is used in watch mode and you want deterministic cleanup when the watcher stops.
2. Ensure these hooks are added alongside any existing hooks (e.g. `name`, `transform`, `handleHotUpdate`, etc.), reusing the `cleanupConsumers()` helper so the logic is centralized and safe to call multiple times.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| this.warn( | ||
| `vite-plugin-rescript: no source map (.map) found alongside ${filePath}. ` + | ||
| `Make sure rescript.json sets "jsx": { "version": 4, "preserve": true } and that source maps are emitted.` | ||
| ); |
There was a problem hiding this comment.
suggestion: Malformed source maps are reported as missing, which can mislead debugging.
In getConsumer, JSON parse failures for .map files also return null, so this warning currently conflates missing maps with malformed ones. That makes it harder to distinguish configuration issues from corrupted/invalid maps. Please either emit a distinct warning for parse failures (e.g., via a separate warnedBadMap set) or adjust this message to mention that the source map may be missing or malformed.
| this.warn( | |
| `vite-plugin-rescript: no source map (.map) found alongside ${filePath}. ` + | |
| `Make sure rescript.json sets "jsx": { "version": 4, "preserve": true } and that source maps are emitted.` | |
| ); | |
| this.warn( | |
| `vite-plugin-rescript: no valid source map (.map) found alongside ${filePath}. ` + | |
| `The source map may be missing or malformed. Make sure rescript.json sets "jsx": { "version": 4, "preserve": true } and that source maps are emitted.` | |
| ); |
| const consumerCache = new Map<string, ConsumerEntry>(); | ||
| const warnedNoMap = new Set<string>(); | ||
| let isDev = false; |
There was a problem hiding this comment.
suggestion (bug_risk): Cached SourceMapConsumers are never destroyed on plugin shutdown.
consumerCache is only cleaned up via invalidateJs/invalidateRes/HMR, so if the dev server stops or a build finishes without those being called, SourceMapConsumer.destroy() is never invoked. This can leak native resources in long dev sessions or large projects. Please add a buildEnd/closeBundle (and possibly closeWatcher) hook to iterate consumerCache.values(), call destroy() on each, and clear the map.
Suggested implementation:
export function vitePluginRescript(
options: VitePluginRescriptOptions = {}
): Plugin {
const consumerCache = new Map<string, ConsumerEntry>();
const warnedNoMap = new Set<string>();
let isDev = false;
function cleanupConsumers() {
for (const entry of consumerCache.values()) {
try {
entry.consumer.destroy();
} catch {
// ignore errors during cleanup
}
}
consumerCache.clear();
}
async function getConsumer(jsFilePath: string): Promise<ConsumerEntry | null> {To fully implement the cleanup:
- In the object that
vitePluginRescriptreturns (thePlugin), add lifecycle hooks that callcleanupConsumers():buildEnd() { cleanupConsumers(); }closeBundle() { cleanupConsumers(); }- Optionally
closeWatcher() { cleanupConsumers(); }if the plugin is used in watch mode and you want deterministic cleanup when the watcher stops.
- Ensure these hooks are added alongside any existing hooks (e.g.
name,transform,handleHotUpdate, etc.), reusing thecleanupConsumers()helper so the logic is centralized and safe to call multiple times.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for ReScript by adding a new Vite plugin, @treelocator/vite-plugin-rescript, which remaps compiled JSX positions back to original .res files using source maps. It also includes a new demo application (apps/vite-rescript), a ReScript adapter in the runtime package, and comprehensive Playwright tests. Feedback highlights potential race conditions and memory leaks in the Vite plugin's getConsumer function, suggesting that SourceMapConsumer instances be explicitly destroyed and that Promises should be cached to prevent redundant allocations.
| const consumerCache = new Map<string, ConsumerEntry>(); | ||
| const warnedNoMap = new Set<string>(); | ||
| let isDev = false; | ||
|
|
||
| async function getConsumer(jsFilePath: string): Promise<ConsumerEntry | null> { | ||
| const cached = consumerCache.get(jsFilePath); | ||
| if (cached) return cached; | ||
|
|
||
| const mapPath = jsFilePath + ".map"; | ||
| if (!fs.existsSync(mapPath)) return null; | ||
|
|
||
| let rawMap: unknown; | ||
| try { | ||
| rawMap = JSON.parse(fs.readFileSync(mapPath, "utf-8")); | ||
| } catch { | ||
| return null; | ||
| } | ||
|
|
||
| const consumer = await new SourceMapConsumer(rawMap as never); | ||
| const entry: ConsumerEntry = { | ||
| consumer, | ||
| jsDir: nodePath.dirname(jsFilePath), | ||
| }; | ||
| consumerCache.set(jsFilePath, entry); | ||
| return entry; | ||
| } |
There was a problem hiding this comment.
The getConsumer function has a potential race condition and memory leak. If multiple transform calls for the same file occur concurrently, multiple SourceMapConsumer instances may be created, but only the last one will be stored in the cache, leading to a leak of the previous ones. Additionally, SourceMapConsumer instances in source-map v0.7+ are WASM-based and must be explicitly destroyed. Consider caching the Promise of the consumer to avoid the race condition and ensure that every created consumer is eventually tracked and destroyed.
| } else if (file.endsWith(".res.js.map")) { | ||
| invalidateJs(file.slice(0, -".map".length)); | ||
| } | ||
| }, |
There was a problem hiding this comment.
The plugin should implement a cleanup mechanism to destroy all cached SourceMapConsumer instances when the Vite process ends or the plugin is disposed. Failing to do so can lead to memory leaks due to the WASM resources used by source-map.
},
buildEnd() {
for (const entry of consumerCache.values()) {
entry.consumer.destroy();
}
consumerCache.clear();
},
Captures the verification done against the spec: plugin behavior, test coverage, what was verified locally (unit tests + build), what wasn't (e2e Playwright run, real rescript compiler), and follow-ups (missing README, undocumented esbuild loader config).
2 participants
Summary
Introduces
@treelocator/vite-plugin-rescript, a Vite plugin that remaps ReScript-compiled JSX positions back to the original.ressource files via source maps. This enables TreeLocatorJS and React DevTools to display correct file/line information for ReScript components.Key Changes
New package:
packages/vite-plugin-rescript/with full implementation.res.jsfiles (which contain preserved JSX).res.js.mapsource maps to remap JSX element positions back to.resfiles__sourceattributes into JSX elements with absolute file paths and line numbersmake.displayName = "ModuleName"to top-levelmakedeclarations so React shows the module name instead of the literalmakeComprehensive test suite:
packages/vite-plugin-rescript/src/index.test.ts.res.jsfiles)__sourceinjection with correct remappingdisplayNameinjection and deduplicationDemo app:
apps/vite-rescript/Button.res.js,Card.res.js+ source maps)scripts/build-fixtures.mjs) to regenerate fixtures from.ressources.res.jsfilesE2E tests:
apps/playwright/tests/ancestry/rescript.spec.tsmake).resfiles, not.res.jsImplementation Details
SourceMapConsumerinstances per.res.jsfile for performanceinjectSourceoption).resfiles changeSource-map status / follow-up
Manual testing against stable ReScript compiler output found that current stable ReScript does not emit sibling
.res.js.mapfiles in the tested setups. Stablerescript@12.2.0also ignores the proposedsourceMapconfig as an unknown field. Details are recorded indev-notes/2026-05-16_rescript-real-compiler-output.md.The source for treating native ReScript source maps as planned/experimental future work is the upstream ReScript PR rescript-lang/rescript#8393, "Introduce Sourcemap". The related ReScript forum feedback thread, Experimental ReScript source map support, says to install a compiler build from that PR, enable a
sourceMapconfig, and notes that it currently generates linked Source Map v3 files such assrc/App.js.map. The same thread also calls out that Vite needs a plugin to pass the generated input maps through, with this reference config: mununki/rescript-sourcemap-test vite.config.js.Until upstream native source-map support lands in a stable ReScript release, this PR should be considered fixture-backed / experimental rather than verified support for stable real ReScript projects.
https://claude.ai/code/session_017tC3BAbezWe1xDaexUR4V6