analogjs / analog

The fullstack meta-framework for Angular. Powered by Vite and Nitro
https://analogjs.org
MIT License
2.55k stars 243 forks source link

Blog Pages Generated By Content Path Have Flicker #1038

Closed jdgamble555 closed 6 months ago

jdgamble555 commented 6 months ago

Please provide the environment you discovered this bug in.

  1. Clone the Analog Project
  2. Run pnpm i
  3. Run 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 the of('') 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.

https://github.com/analogjs/analog/issues/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

jdgamble555 commented 6 months ago

@brandonroberts - I think we have two options here after doing some research.

  1. Put everything in a resolver. The problem with this is that it gets rid of the ability to pass the markdown context to:
    <analog-markdown content="" />
  2. Wait (maybe even a year) for PendingTasks to be public as mentioned here.

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 analog-markdown component with resolvers... for now.

J

brandonroberts commented 6 months ago

@jdgamble555 Yea, I don't want to have to require a resolver because the analog-markdown component should be usable without it.

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.

jdgamble555 commented 6 months ago

Is it possible to use the private ΘPendingTasks?

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.