fable-compiler / Fable

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

Rewrite Fable.CLI #3725

Open MangelMaxime opened 9 months ago

MangelMaxime commented 9 months ago

Description

Current code of Fable.CLI is difficult to maintain.

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

nojaf commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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:

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 commented 8 months ago

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:

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 commented 8 months ago

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>

nojaf commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago
  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:

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 commented 8 months ago

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?"

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 commented 8 months ago

@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.

2) 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.

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

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

MangelMaxime commented 8 months ago

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:

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: ```fsproj netstandard2.0 1.1.0 ``` 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 commented 8 months ago

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 commented 8 months ago

That is a really good take @ncave!

MangelMaxime commented 8 months ago

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 commented 8 months ago

@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.