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

[<AttachMembers>] not compatible with f# member this.Item #3571

Closed
Freymaurer opened this issue Oct 27, 2023 · 8 comments · Fixed by #3573
Closed

[<AttachMembers>] not compatible with f# member this.Item #3571

Freymaurer opened this issue Oct 27, 2023 · 8 comments · Fixed by #3573

Comments

@Freymaurer
Copy link

Description

Use [<AttachMembers>] to improve js syntax, member this.Item ... to make classInstance.[0] syntax accessible.

Repro code

module Class

open Fable.Core
open System.Collections.Generic

[<AttachMembers>]
type MyType() =
    member val MyNumbers = ResizeArray [0..100] with get, set

    member this.Item 
        with get(index) = 
            this.MyNumbers.[index] 

let x = MyType()

x.[0] |> printfn "%i"
// Uncaught TypeError: x.get_Item is not a function

Link to the REPL.

Expected and actual results

Returned js code implements Item member, but calls x.get_Item(0) for x.[0], instead x.Item(0)

    Item(index) {
        const this$ = this;
        return this$.MyNumbers[index] | 0;
    }

The cleanest solution in my opinion would be to enable js array access syntax for classes implementing the Item member. But i am not sure if this can be done.

x[0]
@ncave
Copy link
Collaborator

ncave commented Oct 27, 2023

@Freymaurer Just tested it with latest Fable (4.4.1), seems to work fine:

(function () {
    const arg = x.Item(0) | 0;
    toConsole(printf("%i"))(arg);
})();

I guess the REPL is running an older Fable build, so it will resolve itself once it's refreshed.
Feel free to reopen if you still have issues.

@ncave ncave closed this as completed Oct 27, 2023
@ncave
Copy link
Collaborator

ncave commented Oct 27, 2023

But yes, indexers do seem to be broken when using [<AttachMembers>].
What doesn't work at the moment is this:

[<AttachMembers>]
type MyType() =
    let myNumbers = ResizeArray [0..100]

    member _.Item
        with get (index) = myNumbers[index]
        and set (index) (value) = myNumbers[index] <- value

let main () =
    let x = MyType()
    x[0] <- 5
    x[0] |> printfn "%i"

Where the call to the getter will actually call the setter, without errors (cause JavaScript) but it's incorrect.
I'll keep it open until it's fixed.

@ncave ncave reopened this Oct 27, 2023
@Freymaurer
Copy link
Author

@Freymaurer Just tested it with latest Fable (4.4.1), seems to work fine:

Ah yes! after updating fable my tests pass! Thanks!

Where the call to the getter will actually call the setter, without errors (cause JavaScript) but it's incorrect.
I'll keep it open until it's fixed.

Good to know! Thanks for looking into this!

@Freymaurer
Copy link
Author

Freymaurer commented Nov 6, 2023

Where the call to the getter will actually call the setter, without errors (cause JavaScript) but it's incorrect.
I'll keep it open until it's fixed. @ncave

I just ran into this exact issue again 😅 Can't post a REPL as it is still v4.1.*, but here is the code i used for testing:

#r "nuget: Fable.Core, 4.2.0"

open Fable.Core

[<AttachMembers>]
type TestClass(?arg: seq<string>) =
    let arg = 
        arg 
        |> Option.map ResizeArray 
        |> fun opt -> defaultArg opt (ResizeArray())
    member val StringCounter = arg with get, set
    member this.Item
        with get(index:int) =
            printfn "GET" 
            this.StringCounter.[index]
        and set(index:int) (v:string) = 
            printfn "SET" 
            this.StringCounter.[index] <- v

let testInstance = TestClass([for i in 0..5 do yield $"{i}"])

testInstance[5]
testInstance[3] <- "Hello World"

Both getter and setter transpile to Item with different args. Therefore the setter still shadows the getter and only the setter is called:

image

@ncave
Copy link
Collaborator

ncave commented Nov 6, 2023

@Freymaurer It's already fixed by #3573, will be in the next release.

@Freymaurer
Copy link
Author

So in 4.4.2? Do you know when this will be released?

@ncave
Copy link
Collaborator

ncave commented Nov 7, 2023

@Freymaurer

So in 4.4.2? Do you know when this will be released?

I don't think we have a fixed cadence for releases, so I'm assuming it will be when we accumulate enough changes to do a release.

++ @MangelMaxime can correct me if I'm wrong.

@MangelMaxime
Copy link
Member

Hello @Freymaurer @ncave ,

indeed, we don't have a fixed cadence for the release. In general, we wait to have enough changes or a fix for a nasty bug before releasing.

We could release a new version each time we make a changes but I feel like it would be less impactful and would also generate a lot of noise in term of tweets etc.

With that said, I released a new version of Fable (4.5.0) which should fix the problem reported in this issue.

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

Successfully merging a pull request may close this issue.

3 participants