You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf: split Fable.Transforms into per-target projects for faster incremental builds - #4720
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.
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.
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
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.
By splitting
Fable.Transform.fsprojinto 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.Clion a warm build:FSharp2Fable.fs)Replacements.fs~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.