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

Shorthand syntax for static member no longer works #3675

Open
drhumlen opened this issue Dec 20, 2023 · 6 comments
Open

Shorthand syntax for static member no longer works #3675

drhumlen opened this issue Dec 20, 2023 · 6 comments

Comments

@drhumlen
Copy link

Description

The shorthand syntax for static (static Origin = { x=0; y=0 } vs static member Origin ...) no longer works. This is a breaking change and breaks a lot of our code that relies on this terser syntax.

Repro code

type Currency =
    | EUR
    | USD
    static fromString =
        function
        | "EUR" -> EUR
        | "USD" -> USD
        | _ -> failwith "Invalid currency"

Expected and actual results

It should parse just like:

type Currency =
    | EUR
    | USD
    static member fromString =
        function
        | "EUR" -> EUR
        | "USD" -> USD
        | _ -> failwith "Invalid currency"

Related information

  • Fable version: 4.9.0, but it also breaks for 4.8.1 and 4.8.0 (but works for versions 4.7.0 and lower)
  • MacOS
  • The same (shared code) compiles just fine in the backend running dotnet 8
@drhumlen drhumlen changed the title Shorthan syntax for static member no longer works Shorthand syntax for static member no longer works Dec 20, 2023
@MangelMaxime
Copy link
Member

I suspect this is because Fable 4.8 had a bump in the FCS version used internally.

That FCS bump also caused #3658

@ncave Should we try to use a version of FCS which match with the current released version of it?

@ncave
Copy link
Collaborator

ncave commented Dec 20, 2023

This short syntax doesn't actually compile on .NET 6.0, and is incorrect according to the F# documentation, where member in static member is not optional (cause you can also have static let, static do, static val).

(Although, confusingly, it is unclear why omitting member is allowed for abstract, perhaps cause it's unambiguously always a member?).

Looks like there was a regression that allowed it to compile on .NET 7.0 and .NET 8.0, which was recently fixed.

We could revert this fix for Fable until it shows up in an officially released version of F# (other than .NET 6.0), but it will eventually make it in, as it is already merged in dotnet/fsharp.

@MangelMaxime
Copy link
Member

Thank you for the detailed explanation.

This is a tricky decision, I think we should wait to see what the F# team answer to your comment on their issue. And follow their decision.

@smoothdeveloper
Copy link
Contributor

IMO, we shouldn't modify the compiler or language, if not through a suggestion for such important syntax adjustment.

I feel the argument for making member optional with static has some ground, despite I'm not in favor of it, but then it should be a normal suggestion.

The code which is wrong for all version of F# but version 7 (because nobody reported the issue before) can probably be adjusted with less efforts, despite it is unwelcome surprise, or "breaking".

At least, I'll not let guilt come in my vicinity for having reported the issue, while having compassion for those that liked the member being optional and those that need to adjust code 🙂.

Adjusting the release notes of Fable mentioning that for now, the adjustment is necessary, and pointers to the issue for tracking the status seems to be the best action to take now, so people won't complain of breaking change that isn't described in the release notes.

@ncave
Copy link
Collaborator

ncave commented Dec 20, 2023

@smoothdeveloper

we shouldn't modify the F# compiler or language

Absolutely, but given that the problem is already there in both latest released .NET 7.0 and .NET 8.0, it could be a valid option to rollback the fix temporarily in Fable until it is officially released upstream.

But I agree with all your points, we could also update the Fable release notes to mention it.

@MangelMaxime
Copy link
Member

The code which is wrong for all version of F# but version 7 (because nobody reported the issue before) can probably be adjusted with less efforts, despite it is unwelcome surprise, or "breaking".

I believe version 8 is also impacted.

But yes, I agree that the "correct code" right now, is to use static member as this is what is described in the F# language / RFC etc.

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

4 participants