fix(all): recompile when a source file is added or its output is missing - #4870
Draft
dbrattli wants to merge 2 commits into
Draft
fix(all): recompile when a source file is added or its output is missing#4870dbrattli wants to merge 2 commits into
dbrattli wants to merge 2 commits into
Conversation
Adding a `<Compile Include>` through an imported .props could leave a build broken while still reporting success. Two caches were involved and neither noticed the new file: Retrieving project options from cache, in case of issues run `dotnet fable clean` … Project and references (2 source files) parsed in 123ms Skipped compilation because all generated files are up-to-date! Exit code 0, but the new module was never generated and the code that referenced it was left calling into nothing. The project options cache was validated by comparing the timestamps of the .fsproj and its references. A .props contributes `<Compile Include>` items without any .fsproj being touched, so the cached — and now stale — source list was reused. Adding the file directly to the .fsproj always worked; only the imported case was affected. Collect the files that take part in a project's MSBuild evaluation (transitive `<Import Project>`, plus Directory.Build.props/targets and Directory.Packages.props found upwards) and invalidate on those too. Separately, `areCompiledFilesUpToDate` reported a source whose output file does not exist as up-to-date. `getFilesToCompile` selects exactly those files for compilation — "if files have been deleted, we should likely recompile" — so the two disagreed, and the skip won. Deleting a generated file and rebuilding therefore did nothing at all, no .props required. Treat a missing output as out-of-date. That costs the skip-compilation shortcut to projects containing a file whose generated code is empty, since those are never written to disk and are now indistinguishable from a file that was never compiled. Losing a shortcut is recoverable; silently shipping a half-generated build is not. Both cases are covered by integration tests that fail without this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dbrattli
marked this pull request as draft
August 4, 2026 05:29
MangelMaxime
reviewed
Aug 4, 2026
MangelMaxime
reviewed
Aug 4, 2026
Annotate `string<char>` for the Ionide analyzer and condense the comments added by this PR to 1-3 lines each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
Adding a
<Compile Include>through an imported.propscould leave a build broken while still reporting success —Skipped compilation because all generated files are up-to-date!, exit code 0, new module never generated. The failure surfaced far from its cause (Python:TypeError: exceptions must derive from BaseException; Beam:'Scalars' is not defined). Bad for CI: a pipeline that adds a file and reuses a warm cache green-lights a build whose new code was never compiled.Two independent defects, both reproduced from source.
1. The project options cache ignored imported .props/.targets
Cache validity compared only
.fsprojand reference timestamps. A.propsadds<Compile Include>items without any.fsprojbeing touched, so the stale source list was reused. Adding the file directly to the.fsprojalways worked.Fixed by also invalidating on the files taking part in a project's MSBuild evaluation: transitive
<Import Project>, plusDirectory.Build.props/.targetsandDirectory.Packages.propsfound upwards. Only$(MSBuildThisFileDirectory)and$(MSBuildProjectDirectory)are expanded — an import path built from any other property is skipped, since missing an import costs a stale cache while guessing wrong costs a bogus path.2. A missing output file counted as up-to-date
getFilesToCompileselects a source whose output does not exist (Main.fs:945), thenareCompiledFilesUpToDatereported that same file as up-to-date (Main.fs:981) and skipped the compilation it had just asked for. No.propsneeded: delete any generated file and rebuild → skipped, exit 0, file still missing.Trade-off
A file whose generated code is empty is never written, so it is indistinguishable from one that was never compiled. Projects containing such a file lose the skip-compilation shortcut — real, not hypothetical (
module Emptyemits no output on JS/Dart/Beam), and measured at ~1.2s recompile instead of a skip. Normal projects still skip (0.23–0.28s), so the added import walk costs nothing measurable. Losing a shortcut is recoverable; silently shipping a half-generated build is not.Verification
Integration tests in
CacheInvalidationTests.fsfor both cases, checked in both directions — they fail without this change and pass with it. The deleted-output fixture has two source files on purpose: with one, the existing "no compiled files found" guard masks the bug.Checked by hand on Python, JavaScript and Beam, which now behave identically: add via
.fsproj, add via imported.props, a.propsshared by two projects built in turn, remove from the compile list, deleted output regenerated.Suites: Python 2458 passed, Integration 42 + 116 passed, fantomas clean.
Not addressed
Found alongside, out of scope here. With the two cache defects fixed, both only surface on a compile that already exits non-zero and prints the error — noisy rather than silent.
a.pywritten anyway). This is what poisons the output tree so the next run takes the skip path. Fable compiles each file as it type-checks, and project-wide diagnostics only arrive atFSharpCompilationFinished, so gating writes on "no errors" means buffering output or restructuring that pipeline.raise 1— a non-exception — from F#'s error-recovery AST. A symptom of the above: generating code from an AST that failed type-checking.🤖 Generated with Claude Code