search

Zaid-Ajaj / Fable.Remoting

Type-safe communication layer (RPC-style) for F# featuring Fable and .NET Apps
https://zaid-ajaj.github.io/Fable.Remoting/
MIT License
272 stars 54 forks source link

Implement controlled JSON serialization and initial Fable 3 support #188

Closed Zaid-Ajaj closed 3 years ago

Zaid-Ajaj commented 3 years ago

This PR upgrades the Client library to use SimpleJson v3.16.0 which includes Reflection-based JSON serialization mechanism that doesn't rely on the internal structure of the data types and thus can work for both Fable 2.x (current) and Fable 3 (nagareyama, next).

The PR also includes a new build target called IntegrationTestsNagareyama which will compile the tests application and run the integration tests using Fable 3. That is just after running the classic target IntegrationTests (for Fable 2.x) to make sure Fable.Remoting supports both versions of the compiler :smile:

@alfonsogarciacaro The bundle size of the tests project went from 354KB -> 269KB when compiling using Fable 3 :100:

Zaid-Ajaj commented 3 years ago

Nope! I was too optimistic :joy: The implementation works for Fable 2.x (current) but is having trouble with Fable 3 (next), will need a deep dive to figure this one out 🤔

Zaid-Ajaj commented 3 years ago

So far so good, proxy generation and data communication are working OK-ish (needs refinement). Now only the message pack tests are failing but that could be due to byte[] in Fable 3 is no longer UInt8Array. I will wait for @alfonsogarciacaro's answer before proceeding :smile:

Zaid-Ajaj commented 3 years ago

I can confirm that issue message pack tests is not with enabling or disabling the the typed arrays flag from Fable compiler. I also didn't realize that message pack deserialization is working where all tests regarding IBinaryServer pass successfully when compiling using Fable 3, so I am guessing only writing bytes using message pack format is a bit flaky right now. @kerams Maybe if you have time, you can have a look at why the message pack tests aren't working? (in Fable 2.x they are working like they should)

To run the integration tests using Fable 3, simply use the IntegrationTestsNagareyama build target but to run the live integration tests and have the code in watch mode, then you need 3 tabs to run 3 separate processes (assuming npm install and dotnet tool restore have already been executed):

# tab 1 (fable in watch mode)
cd ClientV2Tests
dotnet fable watch src

# tab 2 (webpack to listen to Fable changes)
cd ClientV2Tests
npm run start-nagareyama

# tab 3 (the Suave backend)
cd UITests
dotnet run
kerams commented 3 years ago

I see this when I try to run those tests

> @ build-nagareyama C:\Users\blah\Desktop\remoting\ClientV2Tests
> dotnet fable src && webpack-cli --mode production --config webpack.nagareyama.js

Fable: F# to JS compiler 3.0.0-nagareyama-alpha-008
Thanks to the contributor! @simra
src> cmd /C "dotnet" restore ClientV2.fsproj
  Determining projects to restore...
  All projects are up-to-date for restore.
Parsing src\ClientV2.fsproj...
Initializing F# compiler...
Compiling src\ClientV2.fsproj...
F# compilation finished in 2787ms
Compiled ..\Fable.Remoting.MsgPack\Format.fs
Compiled .fable\Fable.SimpleJson.3.16.0\Json.fs
Compiled .fable\Fable.SimpleJson.3.16.0\TypeInfo.fs
Compiled ..\Fable.Remoting.ClientV2\Types.fs
Compiled .fable\Fable.SimpleJson.3.16.0\TypeCheck.fs
Compiled src\QUnit.fs
Compiled .fable\Fable.Parsimmon.4.0.0\Parsimmon.fs
Compiled ..\Fable.Remoting.ClientV2\Http.fs
Compiled ..\Fable.Remoting.ClientV2\Remoting.fs
Compiled .fable\Fable.SimpleJson.3.16.0\Parser.fs
Compiled ..\Fable.Remoting.IntegrationTests\Shared\SharedTypes.fs
Compiled .fable\Fable.Mocha.2.5.0\Mocha.fs
Compiled .fable\Fable.SimpleJson.3.16.0\SimpleJson.fs
Compiled ..\Fable.Remoting.ClientV2\Proxy.fs
Compiled ..\Fable.Remoting.ClientV2\Extensions.fs
Compiled .fable\Fable.SimpleJson.3.16.0\TypeInfo.Converter.fs
Compiled ..\Fable.Remoting.MsgPack\Write.fs
Compiled ..\Fable.Remoting.MsgPack\Read.fs
Compiled .fable\Fable.SimpleJson.3.16.0\Json.Converter.fs
Compiled src\App.fs
Fable compilation finished in 895ms
Webpack mode: production
Hash: 35310f3ba78e8cb2b96c
Version: webpack 4.39.3
Time: 226ms
Built at: 11. 10. 2020 16:23:05
 1 asset
Entrypoint main = bundle.js
[0] ./src/App.fs.js 578 bytes {0} [built] [failed] [1 error]

ERROR in ./src/App.fs.js 2052:43
Module parse failed: Unexpected token (2052:43)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|                     test_equal(true, arg10_98);
|                     return AsyncBuilder_singleton.Bind(binaryServer.echoFloatWithMeasure(3,14), (_arg102) => {
>                         const arg10_99 = 3,14 === _arg102;
|                         test_equal(true, arg10_99);
|                         return AsyncBuilder_singleton.Zero();

Shame that test failure error messages are not propagated into the appveyor build log.

Zaid-Ajaj commented 3 years ago

Thanks @kerams for trying it out 🙏 this error you are getting is really weird and I can't really reproduce it locally, the build works properly and event the tests that is recognizes as syntax error (in the logs above) is actually passing like it should. It must some weird file caching issue.

In any case, I think I might have a clue as to why the message pack serialization isn't working: the internal serializerCache that works with System.Type as key is having weird key equality issues when it comes to int64 and potentially other types as well. Tracking that issue as of typeof doesn't work properly as a key for Dictionary (Fable 3)

kerams commented 3 years ago

Yeah, int64 and types that fall back to writeBin were my thoughts too, but then I was puzzled by this

√ Message Pack serialization tests / Set16
X Message Pack serialization tests / Set32

though it looks like you've already found the problem in https://github.com/fable-compiler/Fable/issues/2203

Zaid-Ajaj commented 3 years ago

though it looks like you've already found the problem in fable-compiler/Fable#2203

@kerams, Yeah that was the problem. The deserialization is going through and works like it should. Only when comparing the input with the deserialized output, the test fails.

This PR is pretty much done actually, all Fable.Remoting related tests are working like they should, the ones aren't are coming from upstream and already being tracked. I only need to update the Fable 3 version when the issues are fixed and see the failing tests disappear :smile:

Hmm actually one last thing to do: account for the case where typed arrays aren't used in Fable options --typedArrays false (for example when outputting typescript) to properly send binary data 🤔

Shmew commented 3 years ago

You can always make it work even if --typedArrays false by explicitly creating one. Like I do here, or you can even use my newly released Fable.Extras 😉.

Zaid-Ajaj commented 3 years ago

Thanks Cody, that's what I did just in the previous release before Fable 3 defaulted back to typed arrays. What I am thinking of right now is checking if the byte array is already an instance of Uint8Array then do nothing (typed arrays enabled) or create a concrete typed array otherwise (the same way you did)

Zaid-Ajaj commented 3 years ago

Published the goodies :rocket: