fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.87k stars 296 forks source link

Temp csproj gererated by fable break ionide exntension #3719

Open Fubuchi opened 6 months ago

Fubuchi commented 6 months ago

Description

During the compilation process, fable creates a temporary csproj file and runs the dotnet restore on this file. This action might pull an old FSharp.Core version and cause errors in the editor if the F# code uses newer features.

Here is an example of my fsproj

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <LangVersion>8</LangVersion>
        <RootNamespace>react_client</RootNamespace>
    </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="Fable.Browser.Dom" Version="2.16.0" />
        <PackageReference Include="Fable.Core" Version="4.3.0" />
        <PackageReference Include="Feliz" Version="2.7.0" />
        <PackageReference Include="Feliz.Router" Version="4.0.0" />
    </ItemGroup>
    <ItemGroup>
        <Compile Include="src\shared\FelizJsxConvert.fs" />
        <Compile Include="src\components\Hello.fs" />
        <Compile Include="src\Main.fs" />
    </ItemGroup>
</Project>

The list of dependencies from the generated csproj

image

Notice the FSharp.Core version is 4.7.2 while it should be 8.x.x since I am using net SDK 8.

That causes the ionide extension complaints where ever I use string interpolation since it is a F# 5 feature.

image

I need to run dotnet restore after compile fable to correct the F# version.

Another workaround is adding <DisableImplicitFSharpCoreReference>true</DisableImplicitFSharpCoreReference> and <PackageReference Include="FSharp.Core" Version="8.0.101" /> to the fsproj so the generated csproj contains the desired F# version

Expected and actual results

The generated csproj and compilation process should not affect editor toolings.

Related information

baronfel commented 6 months ago

@MangelMaxime where is the temp csproj being generated? I think the safest thing to do here would be to evaluate the fsproj and get the value of the IntermediateOutputPath, then put your generated csproj inside that location. That location is typically hidden from most editor tooling and shouldn't impact the parent project.

Separately, why have a csproj at all? What specifically was buildalyzer not doing for fsharp projects?

MangelMaxime commented 6 months ago

@baronfel The temporary csproj is placed alongside the fsproj.

So you end up with:

| src
    | - MyProject.fable-temp.csproj
    | - MyProject.fsproj

Please takes my history explanations with precaution, I only followed it by reading the PR names or things like that at the time ^^

The origin of why we do that is because at some point Fable had trouble with .NET 7. I think it was using ProjInfo for the parsing but it had issues or missing information when new version of .NET came out. So Alfonso switched to using buildanalyzer.

However, the problem with buildanalyzer is that it is/was really slow against fsproj file. This is what lead to a lot of issues about Fable 3 not supporting .NET 7 because it was "freezing". So Alfonso had the idea of using a temporary csproj file to get the information needs for Fable. This has been done in this PR https://github.com/fable-compiler/Fable/pull/3265 (it also links to some others discussion).

There has been discussion on moving the temporary csproj into another location but we think that we can't do it because if people use MSBuild features then some links could be broken. For example, if people use Directory.Build.props we need the file to be at the correct place to have the complete view of the project.

Something that has been suggested is that with .NET 8, MSbuild now offer access to the "Design Time tasks or information". Which means that we should be able to ask MsBuild the information we needs.

If I am not mistaken the information we are trying to access are:

projOpts,
Seq.toArray result.ProjectReferences,
result.Properties,
result.TargetFramework

Just by reading the code, I am unsure what projOpts are but here is the place where we retrieve the information we want for Fable

https://github.com/fable-compiler/Fable/blob/b898fb6f6870ac1914248cad2b0a2e8e6d46cdb9/src/Fable.Compiler/ProjectCracker.fs#L614-L767


I also believe that the dotnet restore against the csproj breaks Ionide intellisense supports. As I often, need to call dotnet restore against the fsproj manually after Fable started in order for Ionide to works as expected.

A temporary workaround, could be to invoke dotnet restore against the fsproj after we retrieved the information from the csproj. It should restore the obj / bin folder to the correct state.

This is a temporary solution while we investigate how to not use the csproj hack or to use it in a better way. The plans up to now, where to investigate MSBuild design time feature which can with .NET 8. Our main question up to now, is if that will require people to move their project to .NET 8 or not. As Fable is invoked locally, and will probably respect the global.json which could set the version of .NET to another version.

MangelMaxime commented 6 months ago

In #3722, I implemented the workaround of restoring the fsproj after the temporary csproj.

We are currently exploring a solution to not rely on the csproj in #3721, but it can take some times before it is ready.

nojaf commented 5 months ago

Hi @Fubuchi, in the latest version (4.12.2) you can pass --test:MSBuildCracker as argument and that should not generate the temporary csproj file. This is an experimental feature, so I'd be curious if it works for your project.

Fubuchi commented 5 months ago

I tried to build with --test:MSBuildCracker, and no csproj was created during the building process.