-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
Blog Pages Generated By Content Path Have Flicker #1038
Comments
@brandonroberts - I think we have two options here after doing some research.
<analog-markdown content="" />
On a different note, I think the markdown conversion should be handled completely on the server in an endpoint. There is no reason to load up the client with that package. This is a huge bug SSR, but I don't know that it can be fixed without a re-write of the J |
@jdgamble555 Yea, I don't want to have to require a resolver because the I'd even use the private ΘPendingTasks symbol until it becomes public API if that provides a reasonable solution. We can always switch internally to the public API when it becomes available. We could render the markdown on the server and only ship that to the client, but needs some investigation there also. |
Is it possible to use the private The markdown on the server would just be an internal endpoint accepted the markdown data and returning the html. Of course we need to await that too, but would be cleaner. |
Please provide the environment you discovered this bug in.
pnpm i
nx serve blog-app
Navigate here: http://localhost:3000/archived/post1-2024
Here is the official hosted version:
https://deploy-preview-1042--analog-blog.netlify.app/archived/post1-2024
You will see the page load empty first, and then it loads.
First Flicker Problem
There are actually 2 different flicker problems.
The first flicker is due to the Observable Not Resolving in time for the server to load the data. It does not
await
the markdown comopnent to render. At first I thought this was a problem with theof('')
on this line, but even when I set the variable directly to equal the router observable, it does not complete before the server page is generated.Generally, I would use the waitFor function to force the component to resolve the observable (as a promise) on the server in a different zone before rendering, but that does not work either. This is a dire problem as it could effect loading meta tags or schema for SEO purposes. We need to generate on the server.
We need a Zone.js expert for this.
Second Flicker Problem
If you set the
content$
directly and comment out the markdown rendering on this line, you will see it loads (without markdown parsing) on the server in time. However, there is a second flicker.This problem is not as serious, but I believe the second flicker is from the observable running 2x. Generally speaking the server should fetch the data and hydrate the data to the browser so it doesn't have to reparse the markdown component. It should not re-run the observable on the client unless there is a route change.
Should we just remove the observables all together here? I think signals could help simplify some of this code.
Which area/package is the issue in?
content
Description
Here is a direct blog example you can see the problem:
I can't tell if this is a second flicker problem or transition, so may be unrelated:
I just looked a few blogs from Brandon's post:
Please provide the exception or error you saw
One way to fix the problem completely would be to remove the ability to pass the markdown code directly to the markdown component and handle it server side only then load it in a resolver. This would keep the client code bundle smaller. It adds a better UX but worse DX (arguably).
We may not need to worry about the ZoneJS issue as @atscott points out
Pending Tasks
will take over ZoneJS's ability to run async tasks outside a component.#1033
Either way, Flicker Problem #1 needs immediate attention.
Other information
If someone figures out the fix and doesn't want to do the PR, I am willing to do it.
I would be willing to submit a PR to fix this issue
The text was updated successfully, but these errors were encountered: