dotnet-websharper / google.maps

WebSharper bindings to Google Maps API
Apache License 2.0
4 stars 3 forks source link

Build fails with WebSharper 7.0.3 beta3 #3

Open AlexPeret opened 6 months ago

AlexPeret commented 6 months ago

hi!

I'm trying to build WebSharper.Google.Maps targeting WebSharper 7.0.3 beta3 and after removing some stale references from paket.references files, I'm facing this error message:

WEBSHARPERTASK : WebSharper error WS9001: Error running WIG assembly: System.ArgumentException: An item with the same key has already been added. Key: { Assembly = "WebSharper.Google.Maps" FullName = "WebSharper.Google.Maps.Map" } at System.Collections.Generic.Dictionary2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Collections.Generic.Dictionary2.Add(TKey key, TValue value) at WebSharper.Compiler.Reflector.transformClass@215-5.Invoke(Boolean intfAsClass, TypeDefinition typ) at Microsoft.FSharp.Collections.SeqModule.Iterate[T](FSharpFunc2 action, IEnumerable1 source) in D:\a_work\1\s\src\FSharp.Core\seq.fs:line 603 at WebSharper.Compiler.Reflector.trAsm(IDictionary2 prototypes, AssemblyDefinition assembly, Boolean isTSasm) at WebSharper.Compiler.Reflector.TransformAssembly(IDictionary2 prototypes, AssemblyDefinition assembly) at WebSharper.InterfaceGenerator.Compiler.Compile(Resolver resolver, CompilerOptions options, Assembly assembly, String filePath, FSharpOption1 originalAssembly) at WebSharper.InterfaceGenerator.Compiler.Compile(CompilerOptions options, Assembly assembly, String filePath, FSharpOption1 original) at WebSharper.Compiler.CommandTools.RunInterfaceGenerator(AssemblyResolver aR, FSharpOption`1 snk, WsConfig config, LoggerBase logger) at WebSharper.Compiler.FSharp.Compile.Compile@148-3.Invoke(Unit unitVar0) [/home/vagrant/websharper/google.maps/WebSharper.Google.Maps/WebSharper.Google.Maps.fsproj::TargetFramework=netstandard2.0]

I've also removed the net461 reference, leaving only netstandard20.

The screenshot lists (grep command) where this namespace is located: Screenshot_20240102_212230

My (wild) guess is the compiler is having problem resolving module aliases.

Attached, you find the complete log from the dotnet fake build output. buid.log

My setup:

> uname -a
Linux vagrant 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

> dotnet --version
7.0.114

Is there any workaround for this issue?

Thanks, Alex

granicz commented 6 months ago

@AlexPeret Two issues to deal with before even attempting to compile for WS7:

  1. The current bindings are massively outdated - it would be best to redo them for the latest Google Maps release.
  2. To be compatible with WebSharper 7, the binding needs to be made npm-based -> another argument for updating it.
AlexPeret commented 6 months ago

I see, thanks @granicz .

I'm trying to compile a project built with WebSharper 4.7 on netcore 3.1, but as Ubuntu 22.04 doesn't have support to this version anymore, I cannot compile the project. This is the reason I was trying to upgrade to WebSharper 7.

Maybe I should change my strategy, then. I just figured Microsoft has a package repository which seems to provide netcore 3.1 and I will try it, as I cannot build the (original) project with .net 7 or 6.

Regarding the WebSharper.Google.Maps update, should it have a separate git repo? also, are there any instructions on how to do it, specially about the npm-based bindings?

Thanks, Alex

granicz commented 6 months ago

My suggestion would be to 1) continue using this repo, 2) keep the WIG approach and bind the latest version with it, and 3) target WebSharper 6. This definitely should work and is likely the path with the lowest resistance going forward. First step would be to convert the repo to .NET Core, @Jooseppi12 - can you help with that?

The Google Maps API (https://maps.googleapis.com/maps/api/js) is a hosted, monolithic API that mutates (pollutes, really) window and other top-level objects, and from what I can tell, it doesn't really lend itself to the ESM output we expect to generate with WebSharper 7. @Jand42 - what do you recommend here?

AlexPeret commented 6 months ago

@granicz

For the record, I couldn't install netcore31 from Microsoft's package repository on Ubuntu 22.04. Even installing libssl1.1 from .deb file (found on archive), other dependencies like dotnet-targeting-pack-3.1 aren't available.

My suggestion would be to 1) continue using this repo, 2) keep the WIG approach and bind the latest version with it, and 3) target WebSharper 6.

Unless there are no breaking changes, shouldn't 3) come before 2)?

Regarding 2), I did that before on this lib, but didn't finish. If you don't mind, I would like to try it again.

Jooseppi12 commented 6 months ago

My suggestion would be to 1) continue using this repo, 2) keep the WIG approach and bind the latest version with it, and 3) target WebSharper 6. This definitely should work and is likely the path with the lowest resistance going forward. First step would be to convert the repo to .NET Core, @Jooseppi12 - can you help with that?

We don't need to npmify the binding (as we can't really do it anyway), we just need to update the repo to use WS7 and then we can update the underlying binding to the latest version of the Google Maps API

@AlexPeret I will take update the default branch to use WS7 today. Also I will add a better loader as well, that can take the API version for the URL

Jooseppi12 commented 6 months ago

@AlexPeret I've pushed out the WebSharper 7 based version to the master branch

granicz commented 6 months ago

(@AlexPeret So it turns out I was wrong, we can easily bind monolithic, non-npm based libs with WebSharper 7 - pretty much just like before. So feel free to update the bindings via a PR and we will release to NuGet when merged. Thanks!)

AlexPeret commented 6 months ago

(@AlexPeret So it turns out I was wrong, we can easily bind monolithic, non-npm based libs with WebSharper 7 - pretty much just like before. So feel free to update the bindings via a PR and we will release to NuGet when merged. Thanks!)

Thanks @granicz ! I already have it up to date to v3.38 and hopefully it won't take too much effort to bring up it to the latest version.

AlexPeret commented 6 months ago

Hi @Jooseppi12 !

I'm having some trouble building this project on Ubuntu 22.05. Below, you find the steps I'm doing.

I'm using Vagrant with Oracle VirtualBox 7.

preparing the Linux box

mkdir vm-ubuntu-netcore7
cd vm-ubuntu-netcore7
vagrant init bento/ubuntu-22.04

I've added the following configuration to Vagrantfile:

...
config.vm.network "forwarded_port", guest: 5000, host: 5100

config.vm.provider "virtualbox" do |vb|
  # Display the VirtualBox GUI when booting the machine
  vb.gui = true
  vb.memory = "2048"
  vb.cpus = "1"
end
...
vagrant up

setting up the environment

Remote shell to the Vagrant box:

vagrant ssh

At Vagrant box:

sudo apt-get update
sudo apt-get install -y dotnet-sdk-7.0
mkdir websharper
cd websharper
git clone --recurse-submodules https://github.com/dotnet-websharper/google.maps.git
cd google.maps/
sudo apt-get install dos2unix
dos2unix build.sh
chmod +x build.sh

building the solution

As I don't have access to the private repo, I had to comment the following lines at the paket.dependencies file:

//source https://nuget.pkg.github.com/dotnet-websharper/index.json
//source ../localnuget
rm build.fsx.lock
rm paket.lock
dotnet paket install
./build.sh

This will throw the following error:

Script reported an error:
-> TypeInitializationException: The type initializer for '<StartupCode$build_2ACD30D4C693AEC07C341758221CCA704F3730FF1F2D95DF172458552527A564>.$WebSharper.Fake$fsx' threw an exception.
-> TypeInitializationException: The type initializer for '<StartupCode$Fake-Core-Target>.$Target' threw an exception.
-> TypeLoadException: Could not load type 'Microsoft.FSharp.Core.CompilerServices.ListCollector`1' from assembly 'FSharp.Core, Version=5.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.
Hint: To further diagnose the problem you can run fake in verbose mode `fake -v run ...` or set the 'FAKE_DETAILED_ERRORS' environment variable to 'true'
Warning: Paket resolved a FSharp.Core with version '6.0.0.0', but fake runs with a version of '5.0.0.0'. This is not supported.
Please either lock the version via 'nuget FSharp.Core <nuget-version>' or upgrade fake.
Read https://github.com/fsharp/FAKE/issues/2001 for details.
Warning: The fake-runner has not been updated for at least 12 months. Please consider upgrading to get latest bugfixes, improved suggestions and F# features.

Updating Fake to the latest version (6.0.0) won't fix it:

dotnet tool uninstall fake-cli
dotnet tool install fake-cli
./build.sh
---------------------------------------------------------------------
Build Time Report
---------------------------------------------------------------------
Target               Duration
------               --------
WS-GenAssemblyInfo   00:00:03.4434021
WS-BuildDebug        00:00:23.2929480   (Could not load file or assembly 'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. General Exception (0x80131500))
Build                00:00:00           (skipped)
Total:               00:00:26.8482394
Status:              Failure
---------------------------------------------------------------------
Script reported an error:
-> BuildFailedException: Target 'WS-BuildDebug' failed.
-> One or more errors occurred. (Could not load file or assembly 'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. General Exception (0x80131500))
-> FileLoadException: Could not load file or assembly 'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. General Exception (0x80131500)
-> Could not load 'Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.
   This can happen for various reasons:
   - You are trying to load full-framework assemblies which is not supported
     -> You might try to load a legacy-script with the new netcore runner.
       Please take a look at the migration guide: https://fake.build/guide/fake-migrate-to-fake-5.html
   - The nuget cache (or packages folder) might be broken.
     -> Please save your state, open an issue and then 
     - delete 'Microsoft.Build.Framework' from the '~/.nuget' cache (and the 'packages' folder)
     - delete 'paket-files/paket.restore.cached' if it exists
     - delete '<script.fsx>.lock' if it exists
     - try running fake again
     - the package should be downloaded again
   - Some package introduced a breaking change in their dependencies and .dll files are missing in the resolution
     -> Try to compare the lockfile with a previous working version
     -> Try to lower transitive dependency versions (for example by adding 'strategy: min' to the paket group)
     see https://github.com/fsharp/FAKE/issues/1966 where this happend for 'System.Reactive' version 4

   -> If the above doesn't apply or you need help please open an issue!
Hint: To further diagnose the problem you can run fake in verbose mode `fake -v run ...` or set the 'FAKE_DETAILED_ERRORS' environment variable to 'true'
Hint: The fake-runner has not been updated for at least 6 months. Please consider upgrading to get latest bugfixes, improved suggestions and F# features.

Following the instructions above didn't help:

rm -rf ~/.nuget/packages
rm paket-files/paket.restore.cached
rm build.fsx.lock
./build.sh
There was a problem while setting up the environment:
-> Folder /home/vagrant/.nuget/packages/system.security.principal.windows/5.0.0 doesn't exist. Did you restore group Main? Try to delete paket-files/paket.restore.cached and trying again.
Hint: If you just upgraded the fake-runner you can try to remove the .fake directory and try again.

This tip didn't work neither:

rm paket-files/paket.restore.cached
rm -rf .fake
> Could not find referenced assemblies, please check installed SDK and runtime versions
Could not detect any platforms from 'net8.0' in '/home/vagrant/.nuget/packages/system.text.encodings.web/8.0.0/lib/net8.0/System.Text.Encodings.Web.xml', please tell the package authors
Hint: If you just upgraded the fake-runner you can try to remove the .fake directory and try again.

Is net8.0 required to build it? do you have any suggestion?

Thanks!

Jooseppi12 commented 6 months ago

Hi @AlexPeret ! I will set up an ubuntu environment and will go through this. One thing that jumps out to me is the version of Microsoft.Build.Framework it's looking for quite an older version of the package than what is referenced in the lock file for the build script - so that's where I'm going to start investigating.

AlexPeret commented 6 months ago

@Jooseppi12

from the log above:

...
- The nuget cache (or packages folder) might be broken.
     -> Please save your state, open an issue and then 
     - delete 'Microsoft.Build.Framework' from the '~/.nuget' cache (and the 'packages' folder)
     - delete 'paket-files/paket.restore.cached' if it exists
     - ...

just letting you know that there is no 'Microsoft.Build.Framework' in '~/.nuget' folder.

Jooseppi12 commented 6 months ago

@AlexPeret So I have been able to do some changes to make the above process work:

If you pull latest master and follow your instructions (except removing build.fsx.lock) it should now work. It's not the most elegant solution - but it will do until we get to update the build tool chain on our end in a more systematic way.

AlexPeret commented 3 months ago

@granicz @Jooseppi12

I've finished upgrading it to v3.56 (weekly channel), but I have some questions and considerations.

You can find it at this fork.

Classes vs Interfaces

The current version uses the (WIG) Class to represent interfaces. I believe Class/Config was used to make it easy to instantiate the config objects. I've tried to keep this approach for interfaces with "Config" suffixes, but implemented the others with (WIG) Interface. I'm not sure if this is the correct approach.

Namespaces vs typedef

Should a typedef be annotated with (WIG) namespace and comments? if so, how to do it?

For instance, MarkerSetup from Journal Sharing,

    // JournalSharing.fs:

    //TODO: add namespace: google.maps.journeySharing.MarkerSetup
    let MarkerSetup = MarkerSetupOptions + (DefaultMarkerSetupOptions ^-> MarkerSetupOptions)
    // |> ObsoleteWithMessage "Deprecated: Marker setup is deprecated. Use the MarkerCustomizationFunction methods for your location provider instead."

How to implement top level functions?

e.g.: importLibrary

I've implemented the Geometry one, but it didn't work. There is a commented test code at Samples.fs.

What to do with the so called "parts"?

https://developers.google.com/maps/documentation/javascript/reference/places-widget#PlaceAutocompleteElement-Parts

Is there any support to EventListener?

https://developer.mozilla.org/docs/Web/API/EventTarget/addEventListener#listener

By now, I'm doing:

  let EventListener = Class "EventListener"

but I'm not sure if it is correct.

Test coverage and regression test

GMaps seems to change frequently and I think implementing some unit tests would help a lot for regression tests. Would WebSharper.Testing be a good fit here?

Regards, Alex

granicz commented 3 months ago

Thanks @AlexPeret !

Jooseppi12 commented 3 months ago

@AlexPeret

Hey! Thanks for looking at upgrading this package. I recommend creating a PR from your fork, so we can discuss code changes on it.

Namespaces vs typedef

So the way to handle this, would be putting the obsolete warning on the methods/properties that result in this type, so that the

Is there any support to EventListener?

You should be able to inherit from the EventTarget like how we are inheriting here . You'll need to use the T<'T> helper to convert from dotnet type to WIG type.

Classes vs Interfaces

If they are behaving like an interface, it's fine to use the WIG interface for the implementation. WIG changed since GMaps was original created as library for WS, specific Interface was added later on IIRC

How to implement top level functions?

So with WIG everything is grouped by classes essentially, but that does not mean we don't have a way to implement something like this. The way I would probably implement this would be creating a Global class, with static methods on them that are inlining into a global call. Similar to how we are using the inline logic here.

One other approach we also did in this situation is to have an extra library project next to the bindings, which would contain something like this, or an abstraction layer above the binding (like a computation expression if needed for example). Also a secondary project would be the best way to add extension methods to existing types that would call into the binding - but in this case I think the previously mentioned method would do it.

For the Parts question

I might be misunderstanding this, but I think that's a reference guide on how to access some stuff through the generated DOM, I might be wrong here. But the js typing that is mentioned on Google Maps GH page, links this that has no mention of those. So I think that's safe to ignore

Testing

Ultimately we need not just unit tests, but also integration tests for this one - I will figure something out for this and ultimately this should not hold up the upgrade to the latest version

If there are more questions, I'm ready to help

AlexPeret commented 3 months ago

@Jooseppi12

thanks for the detailed answer.

Classes vs Interfaces

I've found an interesting use case: CircleOptions (at Specification.fs).

It must be an Interface as CircleLiteral extends it, but both Circle's constructor and SetOptions use it.

We could use Object Expressions here, but it would require implementing all properties, AFAIK.

An alternative would be to box an anonymous record and unbox it to CircleOptions (ugly, but seems to worked) or just use CircleLiteral, instead.

PR

I've created the PR. Let me know what I should do next.

Thanks, Alex