fable-compiler / fable-import

Fable bindings for JavaScript libraries
Apache License 2.0
33 stars 32 forks source link

Cannot find replacement for Leaflet.LatLngExpressionModule::ofLatLng #65

Open kblohm opened 6 years ago

kblohm commented 6 years ago

Hi, i tried to use the LatLngExpression.ofLatLng function in the Leaflet module and i am getting an error: error FABLE: Cannot find replacement for Leaflet.LatLngExpressionModule::ofLatLng. I suppose that is because the repo is "supposed" to only contain pure bindings and no helper code, but all the functions to convert between various DUs are actual implementations. Or am i doing something wrong here and this should actually just work?

MangelMaxime commented 6 years ago

Do you have a reproduction code so I can look on my side too ?

I have to admit in general I don't use this helpers and prefer to directly use !^ or U3.Case1 in my code or even !!

I took this admit because when you have nested U3, U2, etc. the compiler is kind of lost.

kblohm commented 6 years ago

Sure, this should be a pretty "minimal" repro based on your fable-react-elmish template. I tried to copy this example to fable. If i replace LatLngExpression.ofLatLng its working fine. Tell me if you need the whole project or anything else. Thanks! App.fs:

module App.View

open Elmish
open Elmish.Browser.Navigation
open Fable.Core.JsInterop
open Fable.Import
open Leaflet

importAll "../sass/main.sass"
importAll "../node_modules/leaflet/dist/leaflet.css"
open Fable.Helpers.React
open Fable.Helpers.React.Props
module RL = ReactLeaflet
let mutable markerPos = null
Leaflet.icon?Default?imagePath <- "//cdnjs.cloudflare.com/ajax/libs/leaflet/1.3.1/images/"

type MapModel = {
  Center : LatLngExpression
  Zoom : float
  Marker : LatLngExpression
  Draggable : bool
}

let Create() =
  {
      Center = Fable.Core.U3.Case3 (51.505,-0.09)
      Marker = Fable.Core.U3.Case3 (51.505,-0.09)
      Zoom = 13.0
      Draggable = true
  }

type Msg =
  | MapChange of MapModel

let mapView model dispatch =
  let refMarker o =
    markerPos <- o
  let updatePosition _ =
    let latLng:LatLng = !!markerPos?leafletElement?getLatLng()
    MapChange { model with Marker = LatLngExpression.ofLatLng latLng } |> dispatch
  let toggleDraggable _ = MapChange {model with Draggable = not model.Draggable} |> dispatch  
  div
    [ ClassName "content"; Style [Height 500; Width 500] ]
    [ h1
        [ ]
        [ str "Map page" ]
      RL.map [ RL.MapProps.Center model.Center; RL.MapProps.Zoom model.Zoom; RL.MapProps.Style [Height 500; Width 500] ] 
        [ RL.tileLayer 
            [ RL.TileLayerProps.Url "https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
              RL.TileLayerProps.Attribution "&amp;copy <a href=&quot;http://osm.org/copyright&quot;>OpenStreetMap</a> contributors" ] 
            []
          RL.marker 
            [ RL.MarkerProps.Draggable model.Draggable
              RL.MarkerProps.OnDragEnd updatePosition
              RL.MarkerProps.Position model.Marker
              RL.MarkerProps.Ref refMarker ] 
                [
                   RL.popup [RL.PopupProps.MinWidth 90.0]
                     [ 
                       span [OnClick toggleDraggable] [str (if model.Draggable then "DRAG MARKER" else "MARKER FIXES")]
                     ]
                ]   
        ] 
    ]

let init () : MapModel * Cmd<Msg> =
  Create(), []

let update msg model : MapModel * Cmd<Msg> =
  match msg with
  | MapChange m ->
      m, []

let root model dispatch =

  div [] 
    [
      mapView model dispatch
    ]

open Elmish.React
open Elmish.Debug
open Elmish.HMR

let withReact =
    if (!!Browser.window?__INIT_MODEL__)
    then Program.withReactHydrate
    else Program.withReact

// App
Program.mkProgram init update root
#if DEBUG
|> Program.withConsoleTrace
|> Program.withHMR
#endif
|> withReact "elmish-app"
#if DEBUG
|> Program.withDebugger
#endif
|> Program.run

paket.dependencies:

source https://nuget.org/api/v2

storage:none

nuget Fable.Elmish.HMR
nuget FSharp.Core
nuget Fable.Core
nuget Fable.Elmish.React
nuget Fable.Elmish.Browser
nuget Fable.Elmish.Debugger
nuget Fable.ReactLeaflet
nuget Fable.Leaflet
nuget Fable.Geojson
clitool dotnet-fable

package.json dependencies:

  "dependencies": {
    "babel-runtime": "^6.26.0",
    "leaflet": "^1.3.1",
    "react": "^16.4.0",
    "react-dom": "^16.4.0",
    "react-leaflet": "^2.0.0-rc.1",
    "remotedev": "^0.2.7"
  }
MangelMaxime commented 6 years ago

Indeed I can't make it work.

@alfonsogarciacaro The strange string is the that error speak about Leaflet.LatLngExpressionModule. Note the Module prefix, I guess this is because Leaflet.LatLngExpression refer to the type and in order to avoid name conflict Fable is adding Module prefix.

kblohm commented 6 years ago

I am pretty sure its just because there is no implementation file. IMO that is not even a problem, because you can just use !^ as you suggested. Its just confusing to see it in intellisense and get an exception afterwards. Just removing all the converter functions is probably fine.

alfonsogarciacaro commented 6 years ago

Sorry, I didn't check the issue in detail. The -Module suffix is added because of this attribute (though the F# compiler would have added it anyways to avoid conflicts with the type with the same name). But the actual issue is what @kblohm says: Fable.Leaflet is expected to be a pure binding (i.e. only imports for a native JS library) as the rest of the packages in this repo and not to contain any actual implementation. Fable thinks a library is a pure binding if it doesn't include the sources in a fable folder in the Nuget package (because in fact, this means it has no access to the F# code). In that case, when asked to get the code, it tries to replace it with something (as it happens for BCL and FSharp.Core APIs).

So the solution would be to either remove the helpers from Fable.Leaflet and leave only the imports. Or add the sources the Nuget package. In the last case, it's probably better to move the project out of this repo, because fable-import is intended for pure bindings (ideally autogenerated in most cases) without actual code implementations.

MangelMaxime commented 6 years ago

Ah yes sorry i forgot to check the binding only thing.

I vote to remove it to keep the compilation fast because dll parsing is quicker.

If we really need this thing I would prefer to create a separate Fable.Leaflet.Helpers.fs. and either create a separate nuget or set this package out of this repo.

The problem is that almost any binding generated by ts2fable will have this special helpers if they have Ux thing and so they aren’t bindings any. So it’s not only a decision for this library