SSR support #8

sivakov512 · 2019-06-06T06:01:00Z

fixes #7

sivakov512 · 2019-06-06T06:10:19Z

There are some little problems with this feature:

I noticed that I could not use the @storeon/router and storeon/devtools at the same time, it raises error in browser: index.js:46 Uncaught Error: Unknown Storeon event Symbol()

You can see that state serializing is hard - user should get state, then remove router state and serialize. Full state serialization is much easier for user, but we can't, because Symbol() can not be serialized. Also, deletion of router state raises for TypeScript, because Symbol can not be used as an index type.

Can we remove the use of Symbol? This will make it much easier to use.

gosolivs · 2019-06-06T06:29:47Z

Removal is undesirable. The presence of a symbol separates the user namespace and the service.

sivakov512 · 2019-06-06T06:32:31Z

I understand. But there are many problems with Symbol and not all of them related to ssr (TypeScript limitations, problem with storeon/devtools and redux-devtools).

sivakov512 · 2019-06-06T06:43:05Z

Documentation recommends avoid @ usage in event names. I think we could use something like @storeon-router/navigate for event names in libraries for storeon.

ai · 2019-06-06T10:11:56Z

@sivakov512 why do we want to save and restore state in the task of router SSR?

Router during rehydrating will look at location and will load the same state as it was on the server.

sivakov512 · 2019-06-06T10:29:51Z

@ai, because it is not user friendly and TS will raise error.
And there is still a problem with storeon/devtools, which is not related to SSR.

ai · 2019-06-06T10:31:41Z

@sivakov512 my question is why you need a serialization at all

sivakov512 · 2019-06-06T10:32:57Z

@ai only because it simpler for usage. But this is not critical, yes.

ai · 2019-06-06T10:34:02Z

We already added a custom event and a lot of code for serialization, it is not easy to use. Let’s remove it until we will have a real use case.

sivakov512 · 2019-06-06T10:38:50Z

I'am not understand you.
This is just example for end-users on how they can pass store from server to browser and load correctly. There is not code for serialization in library.

ai · 2019-06-06T10:39:40Z

I am talking why we need to pass the store’s state from the server to the client at all?

sivakov512 · 2019-06-06T10:41:26Z

User may need to do some logic on request (auth, data prefetch for rendering, etc). All this data should be passed to client for correct hydration.

ai · 2019-06-06T10:43:23Z

It is not part of the router.
It highly depends on the project’s logic so restoring the whole state is not the best option, it is better just to pass specific parts of the state.
It is better to tell about store restoring only in “Storeon with SSR” article. Not in router docs.

ai · 2019-06-06T10:44:17Z

BTW, “auth, data prefetch for rendering” is calling “initial state” and it is not part of SSR. You need an initial state without SSR as well.

This is why we have store.on('@init') to load that initial state with user ID and all prefetch data,

sivakov512 · 2019-06-06T10:46:59Z

Yes, my message was only to show how the user can solve problems when using SSR.
That is, all changes to the documentation should be removed altogether?

ai · 2019-06-06T10:48:18Z

Yeap, I think in this case we should too much project’s specific details which will be hard to move to another project. How to load initial state properly should be in special article (and use @init event).

sivakov512 · 2019-06-06T10:52:37Z

Okay, thanks

But what about the other two problems?
This example cannot work in TypeScript and I can't use storeon/devtools

ai · 2019-06-06T10:54:03Z

This example cannot work in TypeScript

You do not need this example if you do not need to restore the state during rehydration.

I can't use storeon/devtools

Why you can’t use storeon/devtools?

sivakov512 · 2019-06-06T10:56:55Z

Why you can’t use storeon/devtools?

this:

I noticed that I could not use the @storeon/router and storeon/devtools at the same time, it raises error in browser: index.js:46 Uncaught Error: Unknown Storeon event Symbol()

You do not need this example if you do not need to restore the state during rehydration.

But I need =) How to get all state, except of some parts in TypeScript?

sivakov512 · 2019-06-06T10:57:53Z

Yeap, I think in this case we should too much project’s specific details which will be hard to move to another project. How to load initial state properly should be in special article (and use @init event).

I removed "docs"

ai · 2019-06-06T10:58:39Z

But I need =) How to get all state, except of some parts in TypeScript?

How what use case?

I noticed that I could not use the @storeon/router and storeon/devtools at the same time, it raises error in browser: index.js:46 Uncaught Error: Unknown Storeon event Symbol()

SHow me full stack trace (index.js of storeon or of router?)

sivakov512 · 2019-06-06T11:07:41Z

How what use case?

When I talking about auth and data prefetching it was my use case. In my case "get all state except router" is simpler than "get only needed parts".

SHow me full stack trace (index.js of storeon or of router?)

index.js:46 Uncaught Error: Unknown Storeon event Symbol()
    at index.js:46
    at index.js:44
    at Array.forEach (<anonymous>)
    at dispatch (index.js:43)
    at Object.dispatch (index.js:37)
    at index.browser.js:46
    at index.js:44
    at Array.forEach (<anonymous>)
    at Object.dispatch (index.js:43)
    at HTMLBodyElement.<anonymous> (index.browser.js:89)
(anonymous) @ index.js:46
(anonymous) @ index.js:44
dispatch @ index.js:43
dispatch @ index.js:37
(anonymous) @ index.browser.js:46
(anonymous) @ index.js:44
dispatch @ index.js:43
(anonymous) @ index.browser.js:89

I get it on every click on the link

ai · 2019-06-06T11:11:26Z

When I talking about auth and data prefetching it was my use case. In my case "get all state except router" is simpler than "get only needed parts".

Pass this data in window.initialData = { user, posts } and then load by store.on('@init', () => ({ user, posts })

index.js:46 Uncaught Error: Unknown Storeon event Symbol()

It is not a Symbol problem. It is a problem of router logic. Some event was triggered before the event listener was subscribed.

sivakov512 · 2019-06-06T11:23:43Z

Looks like there is no changed event to dispatch
https://github.com/storeon/router/blob/master/index.js#L46

gosolivs · 2019-06-06T12:56:25Z

index.js

@@ -34,14 +32,10 @@ function createRouter (routes) {

  return function (store) {
    store.on('@init', function () {
-      store.dispatch(change, parse(loc.pathname, routes))
+      store.dispatch(change, parse('/', routes))


Why use /here. How is the initial state tracked?

Because there is no way to determine what url is requested from the server, but store should be initialized ith initial state.
User should dispatch store manually with current request url.

The same component will be always rendered. Then what profit will we get with this approach?

You think that there is no need to initialize state on the server by default?

How Storeon will render requested page (if users asked for /posts/1 we need to route this URL to render this page)

Мне кажется вполне ок вариант с моками, потому что это будет спрятано в библиотеке. Меня смущает только код в @init - совсем-совсем не хочется в коде сервера работать с window. В качестве решения можно location делать с "/" по умолчанию.

Ещё я переживаю, что какая-то библиотека может по наличию window определять, на сервере она или нет - тогда мы можем сломать работу совсем другого кода. Или мы можем как-то изолировать моки внутри библиотеки?

Впрочем, у меня есть идеи как решить последнее.

В качестве решения можно location делать с "/" по умолчанию.

Ну мы как раз обсуждали, что это не круто, так как из-за / могут запуститься каакие-то модули

Ещё я переживаю, что какая-то библиотека может по наличию window определять, на сервере она или нет - тогда мы можем сломать работу совсем другого кода.

Такие библиотеки не будут работать с большинством SSR решений. Тот же Preact делает SSR вообще через Puppeter.

Ну мы как раз обсуждали, что это не круто, так как из-за / могут запуститься каакие-то модули

Если так не делать, то придется в коде сервера приложения и правда обращаться к `windowё. Чувство прекрасного мне не позволит так делать и я не стану этим пользоваться.
Если других вариантов решений нет (у меня - нет), то можем закрывать этот PR. Возможно, однажды кто-то придет и придумает как это сделать лучше.

Попробуй https://github.com/majo44/storeon-async-router — там и фич больше и мейнтейнер может пойдёт на другой компромисс между чувством прекрасного и реальных проблем.

ai · 2019-06-06T13:25:36Z

index.js

@@ -34,14 +32,10 @@ function createRouter (routes) {

  return function (store) {
    store.on('@init', function () {
-      store.dispatch(change, parse(loc.pathname, routes))
+      store.dispatch(change, parse('/', routes))


How Storeon will render requested page (if users asked for /posts/1 we need to route this URL to render this page)

sivakov512 · 2019-06-06T13:48:22Z

test/index.test.js

-beforeEach(() => {
-  history.pushState(null, null, '/')
-})
-
 it('init state', () => {


Help me, please - why this test pass?

@ai @isolovev

Let’s first discuss this question #8 (comment)

Maybe with new API this error will be fixed automatically

ai · 2019-06-07T11:02:03Z

Right now I see 2 problems with this PR:

We duplicate code. It will reduce maintainability because in the future we can forget to backport changes from one file to another.
It is not obvious how to set location in SSR mode.

It is not the big problems but we have another option—just add docs on how to mock window.location. Maybe this way will be simpler than creating special SSR version?

sivakov512 · 2019-06-07T11:08:08Z

It is not the big problems but we have another option—just add docs on how to mock window.location. Maybe this way will be simpler than creating special SSR version?

It may be hard - for example, now @init trying to get current url from location. How to do it with mock?

ai · 2019-06-07T11:10:12Z

now @init trying to get current url from location. How to do it with mock?

If we will create global.window.location before calling router everything will be work in the same way as it works in browser.

gosolivs reviewed Jun 6, 2019

View reviewed changes

ai requested changes Jun 6, 2019

View reviewed changes

SSR support

86fc32f

sivakov512 commented Jun 6, 2019

View reviewed changes

sivakov512 closed this Jun 7, 2019

SSR support #8

SSR support #8

Uh oh!

Conversation

sivakov512 commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sivakov512 commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gosolivs commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ai commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

ai commented Jun 6, 2019

Uh oh!

sivakov512 commented Jun 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sivakov512 commented Jun 6, 2019 •

edited

Loading

sivakov512 commented Jun 6, 2019 •

edited

Loading

sivakov512 commented Jun 6, 2019 •

edited

Loading

ai commented Jun 6, 2019 •

edited

Loading