Fix removing function argument when argument is unit

[klofberg]

This PR fixes the issue: #4041

[alfonsogarciacaro]

This is tricky, if we do this we generate code that doesn't feel as native because all functions accepting unit will have a useless argument.

I think we were replacing all identifiers with unit value to avoid this situation, not sure why it didn't work in this case

[klofberg]

Do you think it would be possible to remove the argument from the body instead?

[alfonsogarciacaro]

Interesting, I thought we were doing this already but apparently not 😅
I've sent a naive implementation #4044 but it seems to work

[klofberg]

Great :-) I'm new to developing the fable compiler but I would be happy to help. Should I try to add tests? Maybe I can investigate how it affects Rust and Python?

[MangelMaxime]

Should I try to add tests?

Adding tests is a good idea indeed, but it will need to be done @alfonsogarciacaro PR.

We have 2 types of tests:

• "Standard" tests which are located under the tests/JavaScript, tests/Python, etc. folders
• Integration tests which check compiler errors and code generation too under the tests/Integration

If you want to contribute to Fable feel free to ask questions and we will try to help you.

[klofberg]

I'm uncertain how to run the tests? If i run them with dotnet test all tests passes but I assume that they aren't checked with fable? When I try ./src/Fable.Cli/bin/Debug/net8.0/fable tests/Js/Main --exclude Fable.Core --noCache I get a lot of errors.

Is this the best place for me to ask questions or should I use Discord or something else?

[MangelMaxime]

@klofberg When working with Fable you should use the build.sh and build.bat script (run build.sh --help to learn more)

They will give you all the commands that you can use to work with Fable.

In this case, it would be something like build.sh test javascript

I made Amplifying F# session showing how Fable works: https://amplifyingfsharp.io/sessions/2023-12-15/

[klofberg]

Great, thanks! :-)

…

On Tue, 11 Feb 2025 at 21:07, Maxime Mangel ***@***.***> wrote: @klofberg <https://github.com/klofberg> When working with Fable you should use the build.sh and build.bat script (run build.sh --help to learn more) They will give you all the commands that you can use to work with Fable. In this case, it would be something like build.sh test javascript I made Amplifying F# session showing how Fable works: https://amplifyingfsharp.io/sessions/2023-12-15/ — Reply to this email directly, view it on GitHub <#4042 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA5B7DAW2BS7HVML6VHQCD2PJKARAVCNFSM6AAAAABW3QXVJKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJRHE2TKNRVGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alfonsogarciacaro]

@klofberg I'd be more than happy if you cherry-pick the commit (or just copy the code) and continue the PR. An integration test could be a good idea, so we also check that functions with () as argument become argument-less in JS.

To add an integration test you just need to add/edit at project in the tests/Integration/Integration/data directory with the .js.expected files (you can run the tests once first and change the .actual extension). See another project in the same folder as an example.

About Python, it should have the same behavior more or less. The only thing that surprised me is when I checked In Fable repl, Python had unit arguments even if they're supposed to be removed, so I need to investigate this:

Fable/src/Fable.Transforms/Python/Fable2Python.fs

Line 3386 in 5c95930

let args = FSharp2Fable.Util.discardUnitArg args

[klofberg]

Sounds good. Thanks for the input and help! I will look at what I can do.

[klofberg]

@alfonsogarciacaro I've created a new PR #4050 with your code and added a integration test for javascript. I will try to have a look at the Python issue you mention also.