fable-compiler / fable3-samples

Nagareyama (Fable 3) samples
MIT License
48 stars 10 forks source link

Fable 3 nodejsbundle sample upgrade #2

Open delneg opened 3 years ago

delneg commented 3 years ago

I've started looking into nodejsbundle sample, and stumbled into several problems / questions: 1) Fable-loader doesn't seem to be updated for Fable 3, should it be used at all ? 2) Should we use 'dotnet fable' tool ? 3) I didn't find 'Migration to Fable 3' guide on the website, however there is such a guide for Fable 2 (Migrating to Fable 2) Such a guide would be very helpful to update webpack.config.js specifically

delneg commented 3 years ago

It seems like https://github.com/MangelMaxime/fulma-demo/pull/43 may be pointing in the right direction, however i'm not sure if it's something that should be used here or not

MangelMaxime commented 3 years ago

Hello @delneg,

I think that for now, the "guide" is this PR: https://github.com/MangelMaxime/fulma-demo/pull/43

Well, you found it ^^

And can't help much more as I didn't play yet with Fable 3.

delneg commented 3 years ago

I've started work in https://github.com/delneg/fable3-samples/commits/fix/update-nodejsbundle-fable3 But i'm having some problems with return /^\/\//.test(e) && (e = "https:" + e), n.call(this, e, t) ^ TypeError: n.call is not a function

alfonsogarciacaro commented 3 years ago

Thanks a lot for helping to upgrade the samples @delneg! I left a comment here: https://github.com/delneg/fable3-samples/commit/ee4b32272c73d1d4d9629e5a1665956d563d2428#r45061746

In any case, if you're blocked at any point, don't worry too much, just send a WIP PR or mention me in the commit and I'll try to fix that sample :+1:

delneg commented 3 years ago

After I did some investigation & updated the webpack.config.js according to your comments, it seems there's a problem with Fable.Fetch package. Using the version Fable.Fetch 2.2.0 i was able to reproduce the problem with the following code:

open Fable.Core.JsInterop
open Fetch
importSideEffects "isomorphic-fetch"
printfn "%A" (Fetch.fetch "https://random.dog/woof.json" [])
delneg commented 3 years ago

I've tried converting the sample to use Fable.SimpleHttp, but got stuck with another error -

import { runSynchronously } from "./.fable/fable-library.3.0.0/Async.js";
         ^^^^^^^^^^^^^^^^
SyntaxError: The requested module './.fable/fable-library.3.0.0/Async.js' does not provide an export named 'runSynchronously'

with the following code:

open Fable.SimpleHttp
async {
    let! (statusCode, responseText) = Http.get "https://random.dog/woof.json"
    match statusCode with
    | 200 -> printfn "Everything is fine => %s" responseText
    | _ -> printfn "Status %d => %s" statusCode responseText
} |> Async.RunSynchronously

Looks like a lot of changes in Fable 3 :)

Edit: After tinkering with Fable.SimpleHttp for a bit, i've discovered that it has problems with finding XMLHttpRequest running on Node. I guess it's best to figure out why Fable.Fetch doesn't work anymore I've pushed my WIP-s to https://github.com/delneg/fable3-samples/tree/fix/update-nodejsbundle-fable3 Please tell me if that can go into a WIP PR or we should do something else first

alfonsogarciacaro commented 3 years ago

Sorry for the late reply @delneg. I will try to update this sample myself. Async. RunSynchronously is not supported by Fable because it's not possible to make a thread wait for an asynchronous action in JS, although we should have an error at compile time for this :thinking:

delneg commented 3 years ago

I will try to update this sample myself

Thanks

Async. RunSynchronously is not supported by Fable

yes, it would be nice to have it documented or throw an error because right now it seems like that's some "hidden knowledge"

delneg commented 3 years ago

After the merge of interopFableFromJs it seems like it's the only one left.

alfonsogarciacaro commented 3 years ago

Sorry @delneg, didn't have time yet to look into this. But I'm thinking now maybe we don't need this sample as it's not very common to bundle node.js apps, and we can just remove it. What do you think?

delneg commented 3 years ago

Sorry @delneg, didn't have time yet to look into this. But I'm thinking now maybe we don't need this sample as it's not very common to bundle node.js apps, and we can just remove it. What do you think?

That's true, but I think the problem can be hiding something else (possible, some bug in Fable or the way JS code is processed (bundled) I've updated the branch and re-checked if the problem persists - and it does. Anyway, I'm kinda also against completely removing it because there are only 2 samples with babel - reactComponent and nodejsbundle, so if we'll delete it we'll be left with just one.

MangelMaxime commented 3 years ago

@alfonsogarciacaro

At work, I had to ask my self the question about bundling or not our server/node.js App.

After some reading online it seems like it is more common than we think as it's allow to minify the body, remove comments, etc. All this make the parsing of the server code faster and so the startup time etc.

I have working at work, I can try to take a look at the nodejsbundle sample.

alfonsogarciacaro commented 3 years ago

Thanks for sharing your experience @MangelMaxime. That makes sense, I'm also working in a node.js app, IIRC we looked into bundling but had issues with some dependencies and at the end we're publishing it as it, but it'd be super helpful if you could have a look at the sample 🙏