Skip to content

perf: split Fable.Transforms into per-target projects for faster incremental builds - #4720

Merged
MangelMaxime merged 2 commits into
mainfrom
incremental-build
Jul 5, 2026
Merged

MangelMaxime merged 2 commits into
mainfrom
incremental-build

Conversation

@MangelMaxime

Copy link
Copy Markdown
Member

By splitting Fable.Transform.fsproj into target specific project we can get faster incremental build when touching that part of Fable code base.

Timing

Touch one file, dotnet build src/Fable.Cli on a warm build:

Edit Before After
Python codegen 12.4s 5.5s
Rust codegen 11.9s 5.1s
Dart codegen ~12s 4.3s
Beam codegen ~12s 4.0s
PHP codegen ~12s 4.0s
Babel/JS codegen 13.1s 6.3s
Shared core (FSharp2Fable.fs) 12.3s 12.3s (unchanged, expected)
A target's Replacements.fs ~12.5s 12.6s (unchanged, expected)

~55-70% faster for the common case (fixing one target's codegen)

I tried to find idea in how to improve compilation time for replacement API but didn't come up with a good idea yet.

This PR also fixes a long standing "issues" which was that JavaScript code placement was not consistent with the newer targets.

@MangelMaxime
MangelMaxime requested a review from dbrattli July 4, 2026 16:34
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Python Type Checking Results (Pyright)

Metric Value
Total errors 34
Files with errors 4
Excluded files 4
New errors ✅ No
Excluded files with errors (4 files)

These files have known type errors and are excluded from CI. Remove from pyrightconfig.ci.json as errors are fixed.

File Errors Status
temp/tests/Python/test_hash_set.py 18 Excluded
temp/tests/Python/test_applicative.py 12 Excluded
temp/tests/Python/test_nested_and_recursive_pattern.py 2 Excluded
temp/tests/Python/fable_modules/thoth_json_python/encode.py 2 Excluded

@dbrattli

dbrattli commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Code review — split Fable.Transforms into per-target projects

Reviewed the restructure (per-target sub-project split, new Directory.Build.props, Printer.fs moved into core, Babel files relocated to Babel/). No changes requested — it holds up on every axis I checked.

Verified green:

  • Builds. dotnet build Fable.Cli → all six target DLLs, 0 warnings / 0 errors. Fable.Standalone builds (only pre-existing fcs-fable deprecations). dotnet pack Fable.Compiler → nupkg correctly contains Fable.Transforms.Babel.dll + .xml alongside Fable.Transforms.dll.
  • File coverage & order. All <Compile> entries removed from core reappear in exactly one sub-project, none duplicated, intra-project order preserved.
  • Dependency direction (one-way, targets → core). The near-misses check out: Printer.fs in its new earlier position depends on nothing that follows it; core's retained Beam/Replacements.fs uses Fable.Transforms.Naming (not the moved Fable.Beam.Naming), so it needs neither Beam/Prelude.fs nor Beam/Beam.AST.fs; State.fs references no backend; core no longer references Rust.AST and nothing left in core needs it.
  • Packaging is correct-by-design. Fable.Compiler/Library.fs only calls Fable2Babel/BabelPrinter, so referencing only the Babel sub-project is right; multi-target dispatch lives in Fable.Cli, which references all six. PrivateAssets="all" still copies the DLL to output, so the pack pattern works (same one already used for Fable.Transforms.dll).
  • Directory.Build.props scope. The two projects needing NuGet FSharp.Core (Rust.AST.fsproj, Rust.AST.Tests.fsproj) opt out via DisableImplicitFSharpCoreReference=false, and the test project's own TargetFramework=net10.0 wins over the props' netstandard2.0 (project body evaluates after the imported props).
  • No stale references to the three moved Babel files anywhere else in the repo.

Non-blocking note (not a defect): the pre-PR Fable.Compiler package incidentally bundled the monolithic Fable.Transforms.dll with all codegen types. After this PR the non-JS codegen types are no longer in that package. That's not a supported-API regression (the public API was always JS-only), but any consumer reflectively reaching into the bundled DLL to invoke another target's codegen would break — extremely unlikely and almost certainly intended.

Solid, well-scoped refactor. 👍

🤖 Review generated with Claude Code

@dbrattli dbrattli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@MangelMaxime
MangelMaxime merged commit 260c962 into main Jul 5, 2026
24 checks passed
@MangelMaxime
MangelMaxime deleted the incremental-build branch July 5, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants