Fable: Discussion: Mangle interface method names by default or explicitly

Created on 15 Nov 2016  路  10Comments  路  Source: fable-compiler/Fable

In F#, if two different interfaces both use the method name Foo, there is no conflict. And classes can also use the method name Foo without conflicting with interfaces.

But in Fable, there is a conflict because all of the interfaces and classes compile to the same Foo method in JavaScript. That makes it impossible to use interfaces if there are method name conflicts, unlike in F#.

Assuming that there is an interface IFoo in the module My.Module, one solution is to use My_Module_IFoo_Foo for the interface method name, rather than just Foo. There are two ways of implementing this:

  1. Using a Mangle attribute explicitly. This is already implemented in latest alpha. It solves the conflict, but the attribute must be used explicitly (there'll be a compiler error if the attribute is not used and there're conflicts). Also, it's not possible to add the attribute to interfaces from the BCL (IDisposable, IObservable...).

  2. Making this the default behaviour and creating a NoMangle attribute instead, so the semantics are the same as in standard F#. The main problem is this is breaking and would force changes in several places: republishing all bindings, make sure all code that uses an interface to interact with JS has the NoMangle attribute (either when receiving an object from JS or when sending it from F# as in here, change the Typescript interfaces in fable-core (example), etc.

This is one of those points were Fable needs to decide whether it should remain faithful to F# semantics or rather have an easier interop with JS, so it's important to know the opinion of the community. Taking the second approach will make it easier to share code with .NET, but it'll make migrating to 0.7 more difficult and may increase the cognitive load of Fable users, as they need to remember to use the NoMangle attribute whenever they're defining an interface both for F# and JS.

discussion

Most helpful comment

I've been thinking about this, and apparently the main problem this change would solve is the pattern where a type implements the same method as an interface (to avoid having to cast the object to access it) as in the sample above.

Latest published alpha solves this by mangling the class method instead, which is something already being done for overloaded methods anyways.

This is not perfect as it doesn't solve all issues, like implementing two interfaces with the same method. But those cases are rare and having to modify large parts of fable-core, republish all bindings, update the samples and documentation, add deprecate warnings, deal with bugs and questions because of this, the cognitive load of having to add an attribute when interacting with JS, etc... is too a high a cost to try to cover all possibilities in F# (which is not Fable's ultimate goal), and this doesn't seem to be a high request among Fable users either.

If the current solutions works for you we can close the issue.

All 10 comments

Here are my thoughts:

  • In version 0.7, don't use mangling by default (you can use [<Mangle>] to turn it on).

Change the BCL interfaces to use [<Mangle>]. This mostly just affects fable-core

The compiler should add in deprecation warnings for non-mangled interfaces.

  • In version 0.8, switch to use mangling by default (you can use [<NoMangle>] to turn it off).

[<Mangle>] would become a no-op, because mangling is now the default.

  • In version 0.9, remove the [<Mangle>] attribute.

This should provide a relatively smooth path toward mangling by default (which I support), while remaining pragmatic in the short term.

I鈥檝e just tried to add the CompiledName to an interface member but it doesn鈥檛 work (F# error)

type IInterface1 =
    abstract member Add: int -> int -> int

type IInterface2 =
    abstract member Add: int -> int -> int

type MyClass() = 
    [<CompiledName("Add2")>] 
    member this.Add x y = 
        x + y

    interface IInterface1 with 
        member this.Add x y = 
            x + y

    interface IInterface2 with
        [<CompiledName("Add3")>] // F# Error
        member this.Add x y = 
            x + y

It does work on the normal member and you get the following js:

  MyClass.prototype.Add2 = function Add2(x, y) {
    return x + y;
  };

  MyClass.prototype.Add = function Add(x, y) {
    return x + y;
  };

Could you add a JsCompiledName which acts the same as the CompiledName but without this interface limitation then any name can be chosen, also this would allow IDisposable to be renamed?

From what I can see the Mangle/NoMangle is for a few edge cases that I can't really think of. @Pauan is there a real world example you can give?

I haven't hit a point yet I've needed 2 interfaces on one class (staying away from the monolith etc). I have made a few import files and adding NoMangle all over the place could be a big ask if an edge case can be worked around

@davidtme https://github.com/fable-compiler/Fable/pull/514#issuecomment-260708081

See the above link. It's not just about two interfaces on the same class, it's also about interface methods colliding with class methods, which is a common idiom in F#.

I agree that it is an edge case, since classes/interfaces aren't used that much in F# to begin with.

Keep in mind that ts2fable would automatically add in [<NoMangle>] as appropriate, though that won't help with manually-created import files.

Also keep in mind that right now it's not possible to "workaround" the issue, since you cannot add [<Mangle>] to interfaces that you did not create (including the BCL interfaces). So if two interfaces collide, or if you want to create a subclass which implements a conflicting interface, you're just screwed.

Given the above, an alternate solution is to add in [<Mangle>] support for classes. This would mangle the classes' methods, allowing you to combine a conflicting class and interface. This doesn't solve the issue of conflicting interfaces, and it doesn't solve conflicts when creating a subclass, but it _does_ solve the common case of a class method delegating to an interface.

@Pauan RE: #514 (comment)

Could this work for you;

``` F#
type IFoo =
abstract member Foo : unit -> string

type Foo() =
[ member this.Foo() =
(this :> IFoo).Foo()

[<CompiledName("Dispose_BuildIn")>]
member this.Dispose() =
    (this :> IFoo).Foo()

interface IFoo with
    member this.Foo() =
        "foo"

interface IDisposable with
    member this.Dispose () =
        ignore()

let f = new Foo()
let a = f.Foo()
let b = (f :> IFoo).Foo()

which results in the following JS code:

``` Javascript
export var Foo = function () {
  // Omit Symbol etc

  Foo.prototype.Foo_BuildIn = function Foo_BuildIn() {
    return this.Foo();
  };

  Foo.prototype.Dispose_BuildIn = function Dispose_BuildIn() {
    return this.Foo();
  };

  Foo.prototype.Foo = function Foo() {
    return "foo";
  };

  Foo.prototype.Dispose = function Dispose() {};

  return Foo;
}();
declare(Foo);
export var f = new Foo();
export var a = f.Foo_BuildIn();
export var b = f.Foo();

You could also use the Erase:

``` F#
type Foo() =
member this.Foo() =
"foo"

member this.Dispose() =
    ignore()

interface IFoo with
    [<Erase>]
    member this.Foo() = this.Foo()

interface IDisposable with
    [<Erase>]
    member this.Dispose () = this.Dispose()
resulting in:

``` javascript
export var Foo = function () {
  // Omit Symbol etc
  function Foo() {
    _classCallCheck(this, Foo);
  }

  Foo.prototype.Foo = function Foo() {
    return "foo";
  };

  Foo.prototype.Dispose = function Dispose() {};

  return Foo;
}();
declare(Foo);
export var f = new Foo();
export var a = f.Foo();
export var b = f.Foo();

@davidtme That's a very nice idea! I especially like [<Erase>]:

type Foo() =
    [<Fable.Core.Erase>]
    member this.Foo() =
        (this :> IFoo).Foo()

    interface IFoo with
        member this.Foo() =
            "foo"

I was worried that Fable would throw an error because of conflicting methods, but it doesn't. It seems it's smart enough to understand CompiledName and Erase

I still wouldn't mind [<Mangle>] on classes, but CompiledName and/or Erase is a very good workaround.

Given the above workarounds, I'm now mostly neutral about conflicting interface methods. I still think it would be nice to fix this issue, but the workarounds (Mangle, CompiledName, and Erase) are sufficient for me personally.

@davidtme

I have made a few import files and adding NoMangle all over the place could be a big ask if an edge case can be worked around

I'm curious: do you have an example of where NoMangle would be needed in your import files?

@Pauan Mainly responding to

republishing all bindings, make sure all code that uses an interface to interact with JS has the NoMangle attribute

I鈥檓 guessing - Had to create a cut down version of Fable.Import.Pixi.fs because the current version is targeting v3 not v4. That has interfaces all over the place.

I've been thinking about this, and apparently the main problem this change would solve is the pattern where a type implements the same method as an interface (to avoid having to cast the object to access it) as in the sample above.

Latest published alpha solves this by mangling the class method instead, which is something already being done for overloaded methods anyways.

This is not perfect as it doesn't solve all issues, like implementing two interfaces with the same method. But those cases are rare and having to modify large parts of fable-core, republish all bindings, update the samples and documentation, add deprecate warnings, deal with bugs and questions because of this, the cognitive load of having to add an attribute when interacting with JS, etc... is too a high a cost to try to cover all possibilities in F# (which is not Fable's ultimate goal), and this doesn't seem to be a high request among Fable users either.

If the current solutions works for you we can close the issue.

@davidtme Thanks! You're right, that is a _lot_ of interfaces.


@alfonsogarciacaro Indeed, this issue originally started because I had conflicts between class and interface methods. Mangling class methods does solve the immediate problem, and it was my original suggestion. :+1: Thanks for fixing this!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

alfonsogarciacaro picture alfonsogarciacaro  路  3Comments

jwosty picture jwosty  路  3Comments

tomcl picture tomcl  路  4Comments

et1975 picture et1975  路  3Comments

MangelMaxime picture MangelMaxime  路  3Comments