Fable icon indicating copy to clipboard operation
Fable copied to clipboard

DU simple case equality always false after hot reload

Open kspeakman opened this issue 3 years ago • 4 comments

Description

Using the equal operator = on simple union cases works at first. But after hot reload it always evaluates to false. Contributing factors: The DU type is regenerated on hot reload. But the union instance is kept between reloads. Note: I only tried simple union cases.

Repro code

This can only be observed in a dev environment with hot reload. I created a demo repo:

https://github.com/kspeakman/FableUnionCaseEqualityRepro

image

Expected and actual results

The expression m.Status = On is expected to return true when On and false otherwise. But after hot reload, the expression always returns false.

Related information

  • Fable version: dotnet fable --version = 3.7.18
  • Operating system = Windows 10 Pro 21H2

Other notes

The compiled output of m.Status = On is:

(0,_fable_modules_fable_library_3_7_18_Util_js__WEBPACK_IMPORTED_MODULE_11__.equals)(m.Status, new Status(1))

The issue can be worked around by switching to a match statement:

{m with Transitions = match m.Status with On -> true | _ -> false}

The match statement compiles to:

m.Status.tag === 1

kspeakman avatar Aug 23 '22 04:08 kspeakman

Oh, my. This is a tricky situation. I guess we haven't noticed so far because, as you point out, normally we use pattern matching and in case we only do a tag check. But for equality we do compare the type to make sure the objects don't happen to belong to different unions but have the same structure.

It may not be easy in the current state of the code but we could try to identify the cases where the types are guaranteed to be equal at compile-time (as in this case) and in that case perform only structural equality. This could also solve #2282 which was about a similar problem when comparing unions coming from different bundles.

alfonsogarciacaro avatar Aug 24 '22 04:08 alfonsogarciacaro

That would be awesome. Thank you!

I think this is a very important issue, because equality is a core thing that F# devs rely on. To have it not work sometimes feels pretty scary.

kspeakman avatar Aug 24 '22 10:08 kspeakman

@MangelMaxime I've encountered the same problem during testing Elmish.HMR + Vite.js. The exact problem is with

// used by Union
export function sameConstructor(x, y) {
    return Object.getPrototypeOf(x)?.constructor === Object.getPrototypeOf(y)?.constructor;
}

After each hot reload of a file where the union type is declared, a constructor function is recreated causing equals to fail. Were you able to think about how to resolve it? I guess that @alfonsogarciacaro suggestion:

It may not be easy in the current state of the code but we could try to identify the cases where the types are guaranteed to be equal at compile-time (as in this case) and in that case, perform only structural equality.

is the best one since we can't rely simply on structural equality due to reasons mentioned in #2282.

Another solution could be adding some unique type id (based on location, name and namespace) that could be used together with structural equality.

For those that don't care about checking type but structural equality is enough - Erase attribute should work.

lukaszkrzywizna avatar Dec 11 '23 14:12 lukaszkrzywizna

Another solution could be adding some unique type id (based on location, name and namespace) that could be used together with structural equality.

This would increase the bundle size, so if we do so it would be only in watch mode I think as the problem occurs only with HMR which should be use in watch mode only.

Also, the difficulty is that the equality is described in the Type.ts files and JavaScript doesn't have compilers directive which would allows us to activate / deactivate some features. Some bundlers does support it but this is not a standard feature.

So this would probably means that we have to setup the call to different methods / functions based on the Fable target which can be tricky.

I am unsure what the correct path is here.

MangelMaxime avatar Dec 11 '23 16:12 MangelMaxime