-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
chore(rebuild): properly utilize static html #2074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hydrate should do, i will try to take a look tonight at this |
it doesn’t because we have a promise setting the state internally. We are
also passing a function to Site which will always trigger a new render on
the client.
I tried dozens of stuff but nothing worked. Feel free to try it, I suggest
you branch from this PR.
|
Yeah and using |
I'm completely burnt out around this one, will see if I can tackle on the weekend and with a cleared mind something pops up. |
@jeremenichelli sorry for the delay on this and I see your point. I think there's basically two problems, but the first is the biggest:
The only thing I can think of to solve the first issue is moving all routing to the top-level and mounting the router prior to the rest of the application. By doing this, we could trigger a separate If this doesn't seem like a good approach then maybe dropping the |
@skipjack though I should look at remark and see what it doesn't, I have the feeling that is not going to solve our issue here. I will give it one more try between today and tomorrow to see if I can avoid this, I really want to make sure we did everything we can before jumping into other solutions like Next, nuxt.js or Gatsby. |
Either way, this fix is necessary for properly render static pages and hydrate the DOM, so if it looks good, approave it so we can merge it and take this as base for future improvements. |
@jeremenichelli yeah I wouldn't worry too much about I think I have an idea of how to fix the first (and bigger) issue though. We should be able to pre-load that first bundle in |
Sorry for late reply, going to pull this and test tomorrow |
I'll see if I can avoid hydrate from remounting the whole component too. Someone form us should have a solution, I suggest branching from this PR. |
This change delays the dynamic part of app from mounting until the first page's content has loaded. This prevents the static content from being stripped from the DOM until dynamic bundle is there to replace it. Once we migrate to `remark-react`, the elements won't even be re-mounted as React's diffing algorithm will be used. All other pages are still loaded using dynamic `import()`s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything mentioned should be resolved now between @jeremenichelli's initial work and #2108 which I just merged into here. We could further improve improve rendering via remark-react
which would leverage virtual dom diffing (where as dangerouslySetInnerHTML
does not). Merging this to master
and starting to work on merging some of the other rebuild
related PRs as well.
Great work @skipjack ! |
Initially this PR solves a problem that the html files were generated with
[object Object]
content.That said, we have anothr problem, and it's a BIG one because I know how hard it is solve the server side vs. async content loading issue in React.
Let me show you why. Right now we are creating the pages so they are static and already gather the content for it.
When the bundle hits the browser and runs, it detects the Promise and cleans the content state whiping out the server side generated content and remounting it again.
This is serious because if we don't solve it we might need to move to full SPA which for a documentation site I don't recommend, or move to another static generator like Gatsby, Next or Nuxt.