Skip to content

Commit 0a46297

Browse files
committed
Fix the transform sourcemap signal and path normalisation
Two hook-contract bugs, plus the CI and script gaps found alongside them. The transform hook returned `map: null`, which in the Rollup and Vite contract means "I did not move code, keep the previous mapping". That is false here — the hook replaces F# with JavaScript — and downstream stages took it at face value, generating a map whose `sources` named a `.fs` file while its `sourcesContent` held the compiled JavaScript. Devtools duly showed an F# filename containing JavaScript, which is worse than no map at all. `{ mappings: "" }` is how Vite's own plugins say a mapping was lost. Real F#-to-JS source maps stay blocked on Fable, where FileWriter.AddSourceMapping is a no-op. compileProject built its compiled-output map by iterating the source file list and indexing the daemon's response with an already-normalised path, while fsharpFileChanged normalised the daemon's keys on the way in. The two sets are not the same — signature files are reported as sources and never compiled — and the mismatched lookup would yield undefined for every entry if the daemon ever reported a non-POSIX path. Both paths now key off what the daemon returned. CI ran the plugin tests but never `dotnet test`, so the daemon suite, including the signature-file test added with the hotUpdate work, was not running there at all. Added as its own step in both workflows. Also adds a top-level `ci` script that runs lint, formatting and tests in parallel, for use before committing. The roadmap's claim that `.fsx` files are matched but never compiled was wrong and is dropped rather than acted on: the daemon filters only `.fsi`, so a script listed in the fsproj compiles like any other file, and one that is not listed fails exactly as a stray `.fs` would.
1 parent 7f5fa80 commit 0a46297

7 files changed

Lines changed: 51 additions & 13 deletions

File tree

.github/workflows/main.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ jobs:
4242
- name: Test plugin
4343
run: bun run test:plugin
4444

45+
- name: Test daemon
46+
run: bun run test:daemon
47+
4548
- name: Build daemon
4649
run: bun run --cwd packages/vite-plugin-fable postinstall
4750

.github/workflows/pr.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ jobs:
3535
- name: Test plugin
3636
run: bun run test:plugin
3737

38+
- name: Test daemon
39+
run: bun run test:daemon
40+
3841
- name: Build daemon
3942
run: bun run --cwd packages/vite-plugin-fable postinstall
4043

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727

2828
### Fixed
2929

30+
- The `transform` hook reports `map: { mappings: "" }` instead of `map: null`. `null` claims the previous source mapping still applies, which made later stages emit a map labelling the compiled JavaScript as the contents of a `.fs` file — devtools showed an F# filename containing JavaScript. Real F#-to-JS source maps remain blocked on Fable.
31+
- Compiled output is now keyed off what the daemon returned rather than looked up per source file. The two sets differ (signature files are never compiled), and indexing the daemon's map with an already-normalised path would have yielded `undefined` for every entry had the daemon ever reported a non-POSIX path.
32+
- CI runs the daemon test suite; previously only the plugin tests ran.
3033
- TypeScript `strict` is on. That surfaced a real bug: `configResolved` derived the project directory from `resolvedConfig.configFile`, which is optional, so a project without a Vite config file (or one created programmatically) reached `fs.readdir(undefined)`. It now uses `resolvedConfig.root`, which is always resolved and is also the correct directory when `root` differs from the config file's location. A missing `.fsproj` is now an error rather than a `null` handed to the daemon.
3134
- oxlint warnings fail the lint instead of being reported and ignored.
3235
- `sample-project` runs its scripts on the Bun runtime through its own `bunfig.toml` rather than `bunx --bun` in each script, so the scripts are plain `vite`, `vite build` and `vite preview`. Bun only reads the `bunfig.toml` in the directory a command starts from, so the one at the repo root does not cover it.

ROADMAP.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,11 @@ Order is a rough suggestion: 1-3 are contract fixes and design questions, 4-7 ar
88

99
## 1. Hook-contract fixes
1010

11-
- **`map: null` is the wrong signal** (`index.js:454`). In the Rollup/Vite contract `null` means "I did not move code, keep the previous map"; the transform replaces F# with JS. The correct value is `{ mappings: '' }` — what Vite's own plugins use for this case (`plugins/css.ts:583`, `plugins/asset.ts:247`).
12-
Note: real F# source maps are **blocked upstream**. `FileWriter.AddSourceMapping` in `~/Projects/Fable/src/Fable.Compiler/Library.fs:84-90` is a no-op with the `SourceMapSharp` generator commented out; `CliArgs.SourceMaps` exists but does nothing. Needs a Fable PR first.
13-
Worth knowing what happens meanwhile: because the plugin returns `map: null`, later stages generate their own map, and the served module ends with a `sourceMappingURL` whose `sources` says `Greeting.fs` while its `sourcesContent` is the compiled **JavaScript**. Devtools therefore shows a file named `.fs` containing JavaScript, which is more misleading than having no map at all. `{ mappings: '' }` fixes that on its own.
11+
- **Real F# source maps are blocked upstream.** `FileWriter.AddSourceMapping` in `~/Projects/Fable/src/Fable.Compiler/Library.fs:84-90` is a no-op with the `SourceMapSharp` generator commented out; `CliArgs.SourceMaps` exists but does nothing. Needs a Fable PR first.
12+
The plugin now returns `{ mappings: '' }`, which stops later stages from producing a map that labels the compiled JavaScript as the contents of a `.fs` file, but a real F#-to-JS mapping needs the Fable change first.
1413
- **No `load` hook.** Vite reads the whole `.fs` file off disk purely so `transform` can discard it. A `load` for ids in `compilableFiles` skips the I/O and states the intent. Return `moduleType: 'js'` too — `vite:oxc` does (`plugins/oxc.ts:330`) — otherwise rolldown infers the type from the `.fs` extension.
1514
- **`configuration` is derived from `env.MODE`** (`index.js:364`), so `vite build --mode staging` compiles Debug F#. `state.isBuild` is already captured from `command === "build"` (`index.js:365`) but unused for this. Make it an explicit plugin option defaulting to `isBuild ? Release : Debug`, and document it.
1615
- **`transform.filter` ignores query strings** (`index.js:435`). Vite's convention is `makeIdFiltersToMatchWithQuery` from `@rolldown/pluginutils`, used by Vite itself (`plugins/asset.ts:205`) and by plugin-react. Low impact for the main path — Vite still appends `?import` to bare module URLs (`src/node/utils.ts:308`) but strips it in `transformRequest.ts:497` before the id reaches the plugin container, so `transform` sees a clean absolute path. Explicit queries like `./Component.fs?raw` still fall straight through, and the `compilableFiles.has(id)` lookups should go through `cleanUrl`.
17-
- **Path normalisation is asymmetric** (`index.js:234-238` vs `282-285`). `fsharpFileChanged` normalises the daemon's keys before storing them; `compileProject` indexes `compiledFSharpFiles` with an already-normalised name instead. If the daemon ever returns a backslash path, every value in `compilableFiles` becomes `undefined`. Normalise on the way in, in one place.
18-
- **`.fsx` is matched but never compiled** (`index.js:15`). Script files are never in `compilableFiles`, so every `.fsx` import warns and then fails to parse. Either drop `.fsx` from the regex or handle it.
1916

2017
## 2. The JSX handoff to plugin-react works by accident
2118

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"build": "bun run build:daemon && bun run build:plugin",
1919
"build:daemon": "bun run --cwd packages/vite-plugin-fable build:daemon",
2020
"build:plugin": "bun run --cwd packages/vite-plugin-fable build",
21+
"ci": "bun run --no-exit-on-error --parallel lint fmt test",
2122
"fmt": "oxfmt && dotnet fantomas .",
2223
"fmt:check": "oxfmt --check && dotnet fantomas check .",
2324
"lint": "oxlint --deny-warnings && bun run lint:types",

packages/vite-plugin-fable/src/index.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,13 @@ export function createFablePlugin(
185185
}
186186
const compiledFSharpFiles: Record<string, string> = await requireDaemon().initialCompile();
187187
logInfo("compileProject", `Full compile completed of ${state.fsproj}`);
188-
state.sourceFiles.forEach((file: string) => {
189-
addWatchFile(file);
190-
const normalizedFileName: string = normalizePath(file);
191-
state.compilableFiles.set(normalizedFileName, compiledFSharpFiles[file]);
192-
});
188+
state.sourceFiles.forEach((file: string): void => addWatchFile(file));
189+
// Key off what the daemon returned rather than looking each source file up in it: the two sets
190+
// differ (signature files are never compiled), and indexing a raw-keyed map with an
191+
// already-normalised path silently yields `undefined` for every entry.
192+
for (const [file, javaScript] of Object.entries(compiledFSharpFiles)) {
193+
state.compilableFiles.set(normalizePath(file), javaScript);
194+
}
193195
}
194196

195197
/**
@@ -476,8 +478,10 @@ export function createFablePlugin(
476478
code = oxcResult.code;
477479
}
478480
return {
479-
code: code,
480-
map: null,
481+
code,
482+
// Not `null`, which would claim the previous mapping still holds: this replaced F#
483+
// with JavaScript. `{ mappings: "" }` is how Vite's own plugins say a map was lost.
484+
map: { mappings: "" as const },
481485
};
482486
} else if (state.isBuild) {
483487
// Returning nothing would let Vite parse the F# source as JavaScript, and the user would

packages/vite-plugin-fable/tests/index.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ interface EnvironmentStub {
5656
sent: unknown[];
5757
}
5858

59-
type TransformOutput = { code: string; map: null } | undefined;
59+
type TransformOutput = { code: string; map: { mappings: "" } } | undefined;
6060

6161
interface Harness {
6262
plugin: Plugin;
@@ -286,6 +286,33 @@ describe("transform", () => {
286286
expect(result?.code).toBe("export const sum = 1;");
287287
});
288288

289+
test("signals that the source mapping was lost rather than preserved", async () => {
290+
const h: Harness = harness(
291+
{},
292+
{ sourceFiles: [mathFs], compiled: { [mathFs]: "export const sum = 1;" } },
293+
);
294+
await h.start();
295+
// `null` would tell Vite the previous mapping still applies, and it would then build a map
296+
// claiming the JavaScript is the contents of a `.fs` file.
297+
expect((await h.transform(mathFs))?.map).toEqual({ mappings: "" });
298+
});
299+
300+
test("keys compiled output off what the daemon returned, not the source list", async () => {
301+
// The daemon reports signature files as sources but never compiles them.
302+
const componentFs = `${sampleProject}/Component.fs`;
303+
const componentFsi = `${sampleProject}/Component.fsi`;
304+
const h: Harness = harness(
305+
{},
306+
{
307+
sourceFiles: [componentFs, componentFsi],
308+
compiled: { [componentFs]: "export const c = 1;" },
309+
},
310+
);
311+
await h.start();
312+
expect((await h.transform(componentFs))?.code).toBe("export const c = 1;");
313+
expect(await h.transform(componentFsi)).toBeUndefined();
314+
});
315+
289316
test("returns nothing for an F# file the daemon never compiled", async () => {
290317
const h: Harness = harness({}, { sourceFiles: [] });
291318
await h.start();

0 commit comments

Comments
 (0)