Add Server-Side Rendering (SSR) #306

zaaack · 2018-03-04T13:03:16Z

SSR from fable-react!

This PR is actually working, however, it is still waiting fable-react been published, and elmish-react's withReactHydrate to be merged and published. Currently, it's using paket's GitHub dependences feature.

What's more, here are some breaking changes to add SSR:

Urls are path not hash anymore, which means we would create more routes for each page. Also, click on a direct links would cause a browser refresh which not in the old hash urls, I added a helper function to execute Elmish.Browser.Navigation.Navigation.newUrl directly to avoid it, but not sure it's right. @MangelMaxime
Add jwt to cookies to make SSR support rendering login state, but I didn't remove the jwt in localStorage, this might cause problems...
Because the Server project is now depending on the Client project's files, edit view files would trigger the server to rebuild which takes too long. Perhaps we can optimize the development process, e.g. adding a fake target which only builds Server project when files in the Server folder changes?

zaaack · 2018-03-04T13:48:38Z

Looks I forgot the ui test, I'll fix it tommorow or several days later...

MangelMaxime · 2018-03-04T15:02:58Z

Seems like the helper is needed, other users mention it. This is because of how browser work, the solution used by react, is to create a custom "link" element.

Idea being, the helper is included by default in the custom "link" so usage is transparent.

forki · 2018-03-06T06:57:58Z

can you please change it to the nuget package?

zaaack · 2018-03-06T07:02:09Z

@forki Just planning to do it. 😉

zaaack · 2018-03-06T09:20:29Z

@forki Updated to nuget packages.

I also tried some configuration in dotnet-watch to ignore client files in develop running, but have no luck:

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <PropertyGroup>
    <IsWatch Condition="'$(Configuration)' != 'DebugSSR'">false</IsWatch>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="../Client/ReleaseNotes.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/Pages.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/Style.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/views/Menu.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/pages/Home.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/pages/WishList.fs" Watch="$(IsWatch)" />
    <Compile Include="../Client/pages/Login.fs" />
    <Compile Include="../Client/Shared.fs" Watch="$(IsWatch)" />
  </ItemGroup>
</Project>

Edit: set to false directly will work

<ItemGroup>
    <Compile Include="../Client/ReleaseNotes.fs" Watch="false" />
</ItemGroup>

forki · 2018-03-07T12:31:14Z

build.fsx

@@ -216,6 +217,30 @@ Target "Run" (fun _ ->
 )


+Target "RunSSR" (fun _ ->
+    runDotnet clientPath "restore"


why do we need this? what is it doing?

This will set a compile directive DEBUG_SSR to use webpack-dev-server's bundle file path in the server-rendered path. Ideally, in RunSSR we can trigger client and server to rebuild when we edit any file, so we can dev and debug with SSR; but in normal Run we would only trigger the server to rebuild when we edit back-end files, and we can dev with webpack-dev-server and use the server code as an API service.

But I cannot pass the configuration to dotnet-watch to make it work differently in different command, currently, I totally ignored client files' change when watch run the server...

https://github.com/SAFE-Stack/SAFE-BookStore/pull/306/files#diff-694c6c636cd70bcc23aa716434744995R12
https://github.com/SAFE-Stack/SAFE-BookStore/pull/306/files#diff-3b3c3fd8d4441c71716037ce14cb42b0R14

forki · 2018-03-07T12:32:00Z

can you please rebase/merge with master

zaaack · 2018-03-08T01:26:42Z

@forki Done.

forki · 2018-03-13T06:45:43Z

paket.dependencies

-nuget Fable.Elmish.Debugger
-nuget Fable.Elmish.React
-nuget Fable.Elmish.HMR
+nuget Fable.React 3.0.0-alpha-002


@alfonsogarciacaro any timeline on when these things get released?

The Fable.React 3 stablish version? At the end we're not going to touch more the API so I guess we can do it now (this PR is pending though). I'll have to make some minor but potentially breaking changes for Fable 2, but we can bump another major version then.

For the PR you linked, I think a new nuget should be created.

Bumping the major version for Fable 2, is ok no need to hold the prerelease for it I think.

forki · 2018-03-19T08:38:16Z

nuget Fable.Elmish 2.0.0-beta-2
nuget Fable.Elmish.React 2.0.0-beta-2
nuget Fable.Elmish.Browser 2.0.0-beta-1
nuget Fable.Elmish.Debugger 2.0.0-beta-1
nuget Fable.Elmish.HMR 2.0.0-beta-1

are these things released properly?

@MangelMaxime @et1975 any chance for a release?

forki · 2018-03-19T08:42:53Z

@zaaack what's that package-lock.json file?

zaaack · 2018-03-19T08:44:43Z

@forki I'm afraid not, these prerelease versions are added to using this PR: elmish/react#19
although we can simply add a custom elmish render extension method to remove these.

what's that package-lock.json file?

Probably is added when I ran UITest locally...

forki · 2018-03-19T08:48:58Z

@zaaack yes I'd rather not depend on prereleases for public sample

MangelMaxime · 2018-03-19T08:51:25Z

@zaaack The package-lock.json is in general generared by npm5 command.

Perhaps there is a call somewhere to npm instead of yarn commands.

@forki I am currently porting my work app to the latest prerelease version. Problem, is if we want people to understand how to use this new version we need some more documentation.

@vbfox Should be working on a third article in it Fable + React series, explaining how to write the view functions with Elmish to benefit of react optimization.

IMO, we need to address this lack of documenation before releasing a stable version.

forki · 2018-03-19T08:53:56Z

@MangelMaxime I'm very much in favor of extending docs - but if we don't break stuff then we should still release the versions and extend the samples. It's always bad to have samples on prereleases

forki · 2018-03-19T09:09:53Z

@zaaack this PR does not run SSR on normal build or does it? what hapens if I do just "build.cmd"?

MangelMaxime · 2018-03-19T09:11:46Z

@forki We are breaking stuff :)

The major breaking change being: arguments order has been reversed in view and update function

forki · 2018-03-19T09:12:55Z

yes just saw that. how about minor then? with the hydrate stuff?

MangelMaxime · 2018-03-19T09:17:12Z

@forki It's probably possible, wait for @et1975 answer.

I will try to update Fulma-Demo with support for the new API and good practice stuff. So we will be able to write the documentation needed for the stable release.

zaaack · 2018-03-19T09:27:19Z

@forki Just did some refactoring for RunSSR, sorry that I still cannot pass some parameters to the project conf, but I think the current one is more reasonable.

./build.sh run would open webpack-dev-server automatically, server-side rendering is working but you won't know unless you open the server's url and build the client bundle.js manually.

./build.sh runssr would open the server's url automatically, and run dotnet fable webpack -- -w to build client code, you need to manually refresh your page if you edit your client code, but in this case, you can debug server-side rendering easier.

Neither of these would auto restart your server when you edit your client files.

forki · 2018-03-19T09:38:05Z

well I'm not talking about dev mode. I'm talking about the "build.cmd Deploy"
What would that use SSR if I deploy to azure?

forki · 2018-03-20T12:48:23Z

is it possible to start a command in SSR?

forki · 2018-03-20T13:19:21Z

@zaaack in my app I have to kick of a bunch of actions after the page is served. Like loading a map and showing current pos

zaaack · 2018-03-20T22:14:12Z

@forki Not really, you can start any command you like, but they would be ignored in SSR.

However, here have isomorphicExec and isomorphicView to exec different code or render different view between client-side rendering and server-side rendering in
Fable.Helpers.Isomorphic. You can use this to easily render a loading component in SSR and render the real component in the client.

zaaack · 2018-03-20T22:25:15Z

I'm assuming the map component is written in js native, and imported by FFI, these native component cannot be rendered in SSR. Also including fetch api based on FFI, unless you are using some isomorphic http client library like this:haf/Http.fs#142 If this land I think we can figure out a way to run some initial fetch commands in SSR.

zaaack · 2018-03-21T02:12:41Z

Looks Travis ci need a rerun again.

forki · 2018-03-21T07:12:42Z

@zaaack I don't want them to be rendered server side. I just want that command to spin off after the client loaded the bundle.

zaaack · 2018-03-21T07:19:43Z

@forki Just add commands in your init function, they will be ignored in SSR, but executed in the client.

forki · 2018-03-21T08:01:55Z

oh crazy. that works!

forki · 2018-03-21T08:38:06Z

I think I'm ready to take it in. what are others saying?

forki · 2018-03-21T08:39:09Z

still hoping we can find a way without cookies. It's such a hard thing in the EU

zaaack · 2018-03-21T08:55:46Z

@forki This really depends on your needs. If you don't use cookies to store the login state, then the login state won't be auto carried when requesting pages, so SSR won't get the login state and render it. As far as I know, the most use case of SSR is SEO, pages need login are actually not much need to be rendered on the server side.

For SAFE-BookStore, then there only remains the home page's SSR makes sense if we don't use cookies, I can remove login/wishList's SSR if you want.

forki · 2018-03-21T09:16:48Z

tbh I think that's what we should do. restrict it to when it's logged out.
It's still super valuable

forki · 2018-03-21T10:12:22Z

src/Client/App.fs

-            PageModel = HomePageModel }, 
-        Navigation.newUrl (toHash Page.Home)
+            PageModel = HomePageModel },
+        Cmd.batch [


don't need to batch here

alfonsogarciacaro · 2018-03-21T17:42:48Z

I just had a look at the infamous EU directive on cookies and it says:

Cookies clearly exempt from consent according to the EU advisory body on data protection- WP29pdf include:

authentication cookies, to identify the user once he has logged in, for the duration of a session

Maybe this can be applied to our case? The tricky part here is the duration of the session (persistent cookies do need user consent), would it be ok if the validity of the token is less than 24 hours for example?

theimowski · 2018-03-21T22:22:28Z

This look really great. Wondering whether it should be by default or opt-in in SAFE-Template?

forki · 2018-03-22T07:52:02Z

merged! thank you so much @zaaack !

forki · 2018-03-22T08:31:21Z

https://twitter.com/sforkmann/status/976736877586903042

zaaack force-pushed the ssr branch from 8832e8a to 442dafc Compare March 6, 2018 09:16

zaaack changed the title ~~[WIP] Add Server-Side Rendering (SSR)~~ Add Server-Side Rendering (SSR) Mar 6, 2018

forki reviewed Mar 7, 2018

View reviewed changes

zaaack added 4 commits March 7, 2018 20:47

[WIP] Add SSR

edc21bf

Using nuget packages

a2d28fa

Add ignore client files for dotnet watch

8a432db

Small fixes

a9064c8

zaaack force-pushed the ssr branch from 8f17c88 to a9064c8 Compare March 7, 2018 13:14

forki reviewed Mar 13, 2018

View reviewed changes

Remove package-lock.json

f948f8d

Refactor RunSSR

220f1b4

zaaack added 2 commits March 21, 2018 09:18

Fix typo

094f0e6

Upgrade Fable.Elmish.React to 1.0.2

1f9bbc1

forki closed this Mar 21, 2018

forki reopened this Mar 21, 2018

forki requested review from isaacabraham, Krzysztof-Cieslak and theimowski March 21, 2018 08:38

forki closed this Mar 21, 2018

forki reopened this Mar 21, 2018

Remove cookies

b200e88

zaaack force-pushed the ssr branch from d68d363 to b200e88 Compare March 21, 2018 09:54

forki reviewed Mar 21, 2018

View reviewed changes

Fix review

6e2acb7

forki merged commit e5f9da1 into SAFE-Stack:master Mar 22, 2018

Add Server-Side Rendering (SSR) #306

Add Server-Side Rendering (SSR) #306

Conversation

zaaack commented Mar 4, 2018 • edited Loading

SSR from fable-react!

zaaack commented Mar 4, 2018 • edited Loading

MangelMaxime commented Mar 4, 2018

forki commented Mar 6, 2018

zaaack commented Mar 6, 2018

zaaack commented Mar 6, 2018 • edited Loading

forki Mar 7, 2018

Choose a reason for hiding this comment

zaaack Mar 7, 2018 • edited Loading

Choose a reason for hiding this comment

forki commented Mar 7, 2018

zaaack commented Mar 8, 2018

forki Mar 13, 2018

Choose a reason for hiding this comment

alfonsogarciacaro Mar 13, 2018

Choose a reason for hiding this comment

MangelMaxime Mar 13, 2018

Choose a reason for hiding this comment

forki commented Mar 19, 2018 • edited Loading

forki commented Mar 19, 2018

zaaack commented Mar 19, 2018 • edited Loading

forki commented Mar 19, 2018

MangelMaxime commented Mar 19, 2018

forki commented Mar 19, 2018

forki commented Mar 19, 2018

MangelMaxime commented Mar 19, 2018

forki commented Mar 19, 2018

MangelMaxime commented Mar 19, 2018

zaaack commented Mar 19, 2018 • edited Loading

forki commented Mar 19, 2018

forki commented Mar 20, 2018

forki commented Mar 20, 2018

zaaack commented Mar 20, 2018 • edited Loading

zaaack commented Mar 20, 2018 • edited Loading

zaaack commented Mar 21, 2018

forki commented Mar 21, 2018

zaaack commented Mar 21, 2018

forki commented Mar 21, 2018

forki commented Mar 21, 2018

forki commented Mar 21, 2018

zaaack commented Mar 21, 2018 • edited Loading

forki commented Mar 21, 2018

forki Mar 21, 2018

Choose a reason for hiding this comment

alfonsogarciacaro commented Mar 21, 2018

theimowski commented Mar 21, 2018

forki commented Mar 22, 2018

forki commented Mar 22, 2018

zaaack commented Mar 4, 2018 •

edited

Loading

zaaack commented Mar 4, 2018 •

edited

Loading

zaaack commented Mar 6, 2018 •

edited

Loading

zaaack Mar 7, 2018 •

edited

Loading

forki commented Mar 19, 2018 •

edited

Loading

zaaack commented Mar 19, 2018 •

edited

Loading

zaaack commented Mar 19, 2018 •

edited

Loading

zaaack commented Mar 20, 2018 •

edited

Loading

zaaack commented Mar 20, 2018 •

edited

Loading

zaaack commented Mar 21, 2018 •

edited

Loading