Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite Fable.CLI #3725

Open
MangelMaxime opened this issue Jan 30, 2024 · 19 comments
Open

Rewrite Fable.CLI #3725

MangelMaxime opened this issue Jan 30, 2024 · 19 comments

Comments

@MangelMaxime
Copy link
Member

MangelMaxime commented Jan 30, 2024

Description

Current code of Fable.CLI is difficult to maintain.

We should rewrite Fable.CLI so it is easier to maintain and extends.

  • Rewrite the CLI parser.

    I would like to avoid using Argu as I find that Argu as some limitations in how it allows the CLI to be structured.

    Depending on the time frame, I am interested in exploring using function composition to describe the CLI. It is a similar approach to how Thoth.Json and Fable.Form solve their problem.

    elm-cli-options-parser could be an inspiration for the exposed API.

    This would be released as an independent library.

  • Functions / API should do one thing at the time. For example, currently when we try to retrieve the path of the fable_modules folder, it is not just giving us the path but also tries to delete / initialise the folder which causes issues when running in watch mode.

@nojaf
Copy link
Member

nojaf commented Feb 1, 2024

fable_modules should really only be removed when calling fable clean.
In any other case, like if a package is missing it should address that specific gap.

@MangelMaxime
Copy link
Member Author

fable_modules should really only be removed when calling fable clean.

There 2 others situation where we want it to be removed:

  1. When starting Fable and the user passed --no-cache

  2. If the previous invocation of Fable was made using different arguments.

    For example, if you first invoke fable --target js and then invoke fable --target python we need to invalidate fable_modules because it will contains fable-libraries and libraries code compiled for JavaScript otherwise.

@nojaf
Copy link
Member

nojaf commented Feb 1, 2024

I somewhat disagree with 2, the fable_modules should be organised so that changing targets just means you don't have the files yet for the new target.

Just place everything under fable_modules/javascript/PackageBlah. Removing fable_modules you really be an explicit action like --no-cache or fable clean

@MangelMaxime
Copy link
Member Author

To reword to something more neutral, I am going to say invalidate cache. Invalidating the cache could be done by deleting the folder or by using the folder structure for example.

Case where we want to invalidate the cache:

  • If Fable version changes since the last invocation

  • If Fable was invoked with different "impactful arguments"
    For example, --lang does impact the cache invalidation but --verbose does not.

    TODO: Make a list of all the arguments/options and decide if they are impactful or not.

    Should adding a Fable plugin / failing to load it impact the cache?

  • If Fable is invoked with --noCache

  • If fable clean is invoked

For the --noCache and fable clean, we could do a partial clean up but removing only the cache of the impacted target. But perhaps, this is going a bit too far for not much benefit.

@nojaf
Copy link
Member

nojaf commented Feb 1, 2024

I also feel like the cache means different things in different contexts. It would be beneficial to be able to be explicit about this. I might make wrong assumptions about things as well.

There are multiple aspects here:

  • Fable needs F# compiler arguments (now from Buildalyzer). These define the very input of a project compilation.
  • What Fable does in fable_modules is effectively move some files from referenced NuGet packages to be able to construct a single FSharpProjectOptions instance (it does this in memory).
  • When all files of the project are transpiled to another language, they produce a potentially cachable result as well.

The cache key for all these things will also be a set of different combinations.
I think it would be wise to brainstorm about common scenarios and how they would affect the project state.

@MangelMaxime
Copy link
Member Author

We should consider give a try to using simple-exec instead of our custom wrapper on top of ProcessInfo. This is often something difficult to do correctly so if we can delegate it to another library perhaps this could simplify our code too.

I know that in the past, Alfonso wanted to minimised the number of dependencies Fable but I think if it can simplify our code and make it more robust we should consider using dependencies. Like we did for the logger for example.

Speaking of the logger, I propose that the new CLI have this option --level <normal|verbose|debug>

  • normal is the current behaviour which rewrite the console output to keep it as small as possible
  • verbose logs the same informations as normal but with each logs on a new line
  • debug is the equivalent to the current --verbose so we can retrieve additional information from Fable. We should consider if we want to still log the message Typechecked file ... which I am not sure if it add a log of value. Was probably useful when working on the making parallelised as many tasked as possible.

@nojaf
Copy link
Member

nojaf commented Feb 21, 2024

Having some additional dependencies for Fable.Cli isn't the end of the world.
As this is published as a dotnet tool, all the dependencies end up in the artefact anyway.

In Fantomas we have a few dependencies for the cli tool but are very strict about having none for the library (Fantomas.Core). I would try and follow the same principle for Fable.Cli and Fable.Compiler.

I'm in favour of a wrapper of ProcessInfo. Everybody has this problem and we should indeed pick a good library for this. I always use CliWrap. It looks a bit more popular than simple-exec.

For logging, I would stick with the verbosity levels of MSBuild (q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]). Ok, maybe not have minimal as I don't see how to map that one on a LogLevel.

@MangelMaxime
Copy link
Member Author

In Fantomas we have a few dependencies for the cli tool but are very strict about having none for the library (Fantomas.Core). I would try and follow the same principle for Fable.Cli and Fable.Compiler.

Thanks for sharing your experience.

I'm in favour of a wrapper of ProcessInfo. Everybody has this problem and we should indeed pick a good library for this. I always use CliWrap. It looks a bit more popular than simple-exec.

I don't know about CliWrap, I will have a look at it.

For logging, I would stick with the verbosity levels of MSBuild (q[uiet], m[inimal], n[ormal], d[etailed], and diag[nostic]). Ok, maybe not have minimal as I don't see how to map that one on a LogLevel.

Makes sense to me 👍

Before starting to rewrite Fable.CLI, I will make a proposition for how the CLI tools args/options could be done. Reason for that is at the moment all the options are top level but it doesn't make sense for all the language to have access to them.

This means that we will have a breaking changes in how the CLI works, but hopefully it will allow for a better user experience / discoverability of the options.

@nojaf
Copy link
Member

nojaf commented Feb 21, 2024

I think users will be on board with breaking changes if it improves the user experience.
I'm recently a bit inspired from the friendliness of how Vite, Astro and Bun do it:

image

image

image

@MangelMaxime
Copy link
Member Author

Here is a first draft of a CLI design to start the discussions:

First thing which denote from the current CLI is that I am proposing to handle the target switch as a subcommand instead of a flag. This will allow us to offer better help messages which are specific to each target.

dotnet fable

Description:

    F# transpiler to JavaScript, Python, TypeScript, and more

Commands:

    clean           Clean Fable generated files
    dart            Compile to Dart
    javascript      Compile to JavaScript
    typescript      Compile to TypeScript
    python          Compile to Python
    rust            Compile to Rust

Options:

    --cwd                   Working directory
    --define                Defines a symbol for use in conditional compilation
    -l, --logLevel <level>  Set the log level (default: normal)
                            Values: quiet | normal | detailed | diagnostic
    --noCache               Recompiles all files 
    -o, --outDir <dir>      Output directory
    --run <command>         Run the specified command after the first compilation
    --runWatch <command>    Run the specified command after each compilation
    -v, --version           Show the version number and exit
    -h, --help              Show this help message

dotnet fable javascript

Description:

    Compile F# to JavaScript

Options:

    -h, --help              Show this help message
    -e, --extension <ext>   File extension for output files
                            Default is `.fs.js` unless Output directory 
                            is specified then it's `.js`
    --exec <args>           Execute Node against the last generated file after each compilation
                            It will forward <args> to the Node process                            

Questions:

  1. Should clean be a top level command or a subcommand of each target?

    If it is a top level command, some of the global flags don't applies to it:
    - --noCache
    - --run
    - --runWatch

    Which means that we should probably move these flags to the subcommands.

    However, if clean is a subcommand of each target, then having these flags as global flags makes more sense for all targets.

    Should we just not support dotnet fable clean anymore?

  2. What should we do with options that could be common to all targets but have different descriptions?

    For example, -e, --extension <ext> where the default extension is different for each target.

  3. Should we support watch mode only using a --watch flag or should we also supports

    dotnet fable watch javascript or dotnet fable javascript watch?

    The second one could align with dotnet fable javascrip clean.

@nojaf
Copy link
Member

nojaf commented Feb 22, 2024

As someone who's been using Fable mainly to turn F# into JavaScript, it feels like a step back having to type dotnet fable javascript every time.

Why not let us set this up in the fsproj file? If it's not set, how about using an environment variable to pick a default? Like, if you're all about Python, you could just set an env var once and be done.

  1. What's the deal with the clean command? How's it different from --noCache? Honestly, it's always been a bit of a puzzle to me.

  2. I'm thinking the main command could use a -e flag.

  3. Still deciding, but leaning towards --watch for now.

@MangelMaxime
Copy link
Member Author

  1. What's the deal with the clean command? How's it different from --noCache? Honestly, it's always been a bit of a puzzle to me.

In the current implementation:

  • dotnet fable clean delete .fs.js and fable_modules

  • dotnet fable --noCache only delete fable_modules and always make a full compilation even if the source files didn't change since last run

Personally, I don't like dotnet fable clean people should use git clean -xdf to delete the generated files or rm ...

  1. I'm thinking the main command could use a -e flag.

How would you write the description of this flag as it is dependant on the target.

  1. Still deciding, but leaning towards --watch for now.

I do prefer the flag myself too.

Why not let us set this up in the fsproj file? If it's not set, how about using an environment variable to pick a default? Like, if you're all about Python, you could just set an env var once and be done.

Personally, I am not a big fan of env variable for controlling such behaviours. It is easy to forget that you had this env variable setup and wonder why you don't have the default behaviour in a new project.

It is kind of like the same as when using global tools or local tools. IHMO we should always use local tool because the version wanted is linked to the project and the computer.

Allowing to use MSBuild to configure Fable options could be a good idea and would allows us to do like most tool. Have a CLI interface and a config files.

I am not familiar with MSBuild but is it possible evaluate a project with it and extract information afterwards? What I have in mind is we probably want to support MSBuild evaluation because if we allow setting variables on fsproj people will probably think that they can use files like Directory.Build.props to configure the default options for all their project below that file just like in normal MSBuild project.

If we allow multiple source of configuration in what order should it be?

from highest to lowest priority

  1. CLI arguments
  2. MSBuild properties (in project files)
  3. Env variables

As someone who's been using Fable mainly to turn F# into JavaScript, it feels like a step back having to type dotnet fable javascript every time.

True but the problem is that Fable is not anymore just a JavaScript transpiler and needs to adapt to that. We should provide a similar experience for all the targets.

It is the same with Fable.Core who needs a rewrite because ATM JavaScript API are leaked to others target instead of being sandboxed/scoped.

Perhaps, we could consider dotnet fable and alias to dotnet fable javascript.

And if the users wants to see the options for javascript he needs to write dotnet fable javascript --help.

Another solution is to structure the help message differently by flattening it:

Description:

    F# transpiler to JavaScript, Python, TypeScript, and more

Options:
    --cwd                   Working directory
    --define                Defines a symbol for use in conditional compilation
    -l, --logLevel <level>  Set the log level (default: normal)
                            Values: quiet | normal | detailed | diagnostic
    --noCache               Recompiles all files 
    -o, --outDir <dir>      Output directory
    --run <command>         Run the specified command after the first compilation
    --runWatch <command>    Run the specified command after each compilation
    -v, --version           Show the version number and exit
    -h, --help              Show this help message

Commands:

    javascript (default)
        
        Compile F# to JavaScript

        Options:
            -h, --help              Show this help message
            -e, --extension <ext>   File extension for output files
                                    Default is `.fs.js` unless Output directory 
                                    is specified then it's `.js`
            --exec <args>           Execute Node against the last generated file after each compilation
                                    It will forward <args> to the Node process 

    dart            
        
        Compile to Dart

        Options:
            ...

    python
        
        Compile to Python

        Options:
            ...

    rust

        Compile to Rust

        Options:
            ...

And saying that javascript is the default command. But this makes a long help message which is less focused.

@nojaf
Copy link
Member

nojaf commented Feb 22, 2024

Personally, I don't like dotnet fable clean people should use git clean -xdf to delete the generated files or rm ...

Yeah, that makes sense.

How would you write the description of this flag as it is dependant on the target.

Could you elaborate on that? How is modifying the extension impactful for any particular target?

True but the problem is that Fable is not anymore just a JavaScript transpiler and needs to adapt to that. We should provide a similar experience for all the targets.

There is reality and there is perception. I would be in favour of organizing a twitter poll with the following question: "What do you transpiled F# to?"

  • Only JS
  • Only TS
  • Only Python
  • Only Rust
  • Multiple languages

If the overwhelming majority is still in the JS camp, (which is my perception at the moment) we should take this into account.

It is kind of like the same as when using global tools or local tools. IHMO we should always use local tool because the version wanted is linked to the project and the computer.

Yes, of course, you want a local tool but that doesn't mean that all your different repositories will target a language. As I suspect, JS will be a good default here. Python folks could override this with an env var, if they only do Python.

I see this similar to how the --lang flag works for dotnet cli. If you only do F# thing, you set DOTNET_NEW_PREFERRED_LANG to F#.

I would go for a default command (dotnet fable and not specifying anything) and then select the target based on the default value (JS), user-defined environment variable or MSBUild property. (Last one wins).

If you specify a specific target like dotnet fable python, we don't check anything and just use Python.

I am not familiar with MSBuild but is it possible evaluate a project with it and extract information afterwards?

Yep, if we were to settle on something like FableLanguage we can evaluate that using dotnet msbuild YourProject.fsproj --getPropertyItem:FableLanguage and however MSBuild resolves it, we can use that value.

@ncave
Copy link
Collaborator

ncave commented Feb 22, 2024

@MangelMaxime

Please note this is only a personal opinion/preference.

I have to agree somewhat with @nojaf that if the majority of use cases are for specific language it should be the default. But it would be really nice if we don't introduce yet another new environment variable or MSBuild property that people need to remember or look up every time. My strong preference is for a simple --language option that defaults to JavaScript, e.g. --language Python.

  1. IMO everything besides the project name should be an option/flag with sensible defaults, including --clean. Perhaps even the project name can be a --project option.

About the difference between --clean and --noCache, I agree they're somewhat similar, but perhaps we need both, for backwards compatibility. Ideally the rewrite should map all existing options to the new ones 1:1. Personally I too prefer using git clean, but that depends on having a git repo and .gitignore in the project/folder, which might not always be the case everywhere.

  1. For --extension, the description can just say that, if not specified, the default extension is language specific, or prefixed with .fs when --outDir is also not specified. Something like that.

  2. I prefer --watch, see 1).

Again, this is only a personal preference, but I will side with the majority.

@MangelMaxime
Copy link
Member Author

Oh yes for sure, anything related to how we expose an API or craft a CLI (probably worst for a CLI) is a personal opinion/preference.

I hope, I didn't appear as trying to force something in this discussion. I am trying to gather feedback and to come up with a collective decision.

My way of thinking here was to go with the most "strict design" at first to see the limitations / UX from it. And as seen, it leads to severals questions and probably needs to be made more pragmatic related to Fable target audience and usage.

I based the proposition on Command Line Interface Guidelines


I am now thinking that we perhaps indeed keep dotnet fable as if user asked to target javascript.

And then having dotnet fable python, dotnet fable rust, dotnet fable clean (if kept), etc.

The reason for using subcommand instead of using a flags (--language <target>) for switching the target is that by using subcommand we can have specific help message per target.

For example:

  • --typedArrays, --sourceMaps, only makes senses in the context of JavaScript / TypScript.
  • --yes makes senses only for clean

It would also allows to expose target specific flags. For exemple, I suppose Rust could benefit from flag which features set is supported.

CleanShot 2024-02-22 at 18 09 39


Regarding different source of configuration, I think it could be a nice addition even if like mentioned by @ncave. It means that we need good documentation for it.

To follow Dotnet/MSBuild order we should do:

From highest to lowest priority

  1. CLI arguments
  2. MSBuild properties (in project files)
  3. Env variables
Click to see - How I tested this behaviour

I checked against a fsproj and it seems to align with dotnet/msbuild behaviour.

Tested by having a MyLib.fsproj with this content:

<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <TargetFramework>netstandard2.0</TargetFramework>
       <PackageVersion>1.1.0</PackageVersion>
    </PropertyGroup>
	<!-- .. -->
</Project>
  1. Run dotnet pack and it will generate MyLib.1.1.0.nupkg
  2. Run dotnet pack -p:PackageVersion=2.0.0 and it will generate MyLib.2.0.0.nupkg

I am not familiar with MSBuild but is it possible evaluate a project with it and extract information afterwards?

Yep, if we were to settle on something like FableLanguage we can evaluate that using dotnet msbuild YourProject.fsproj --getPropertyItem:FableLanguage and however MSBuild resolves it, we can use that value.

That's great news then 👍

But in my mind if we go this path, it would not be limited to just the target but all the options:

Pseudo XML, it is just here to illustrate.

  <PropertyGroup>
    <FableGroup>
        <FableLanguage>javascript</FableLanguage>
        <FableOutputDir>../fableBuild</FableOutputDir>
        <!-- 
            In theory we should be able to use MSBuild variables too
            <FableOutputDir>$(MSBuildThisFileDirectory)/../fableBuild</FableOutputDir> 
        -->
        <!-- ... -->
    </FableGroup>
  </PropertyGroup>

@ncave
Copy link
Collaborator

ncave commented Feb 22, 2024

IMO it all depends on who we consider the largest audience for Fable. If it's web developers, IMO we shouldn't add more MSBuild properties or ask them to be fiddling with F# project files too much (besides simply including the F# code files in the project). It's already probably a high bar for them having to deal with .fsproj files, adding more is perhaps counter-productive.

@nojaf
Copy link
Member

nojaf commented Feb 22, 2024

That is a really good take @ncave!

@MangelMaxime
Copy link
Member Author

For reference here is the result of the poll:

CleanShot 2024-02-27 at 20 02 29

TBH I am quite surprised by the number of TypeScript usage.

Regarding Rust, some people commented that they wanted to use Rust but didn't because they didn't know how to interop with existing code / libraries which is expected as we don't have documentation for it.

@ncave
Copy link
Collaborator

ncave commented Mar 1, 2024

@MangelMaxime

For Rust (much like interop with F# assemblies from C#), it probably makes most sense to build the domain logic in F#/Fable and interface it with a native Rust adapter. Or where possible, build everything in F#/Fable.

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

No branches or pull requests

3 participants