Skip to content
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

Parallel rendering does not respect arrays #7127

Closed
1 task
smnbbrv opened this issue May 19, 2023 · 3 comments · Fixed by #7136
Closed
1 task

Parallel rendering does not respect arrays #7127

smnbbrv opened this issue May 19, 2023 · 3 comments · Fixed by #7136

Comments

@smnbbrv
Copy link

smnbbrv commented May 19, 2023

What version of astro are you using?

2.5.0

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

I tested the version 2.5.0 with parallel component rendering with the setup where I have originally discovered the parallel rendering issue and it does not seem to be fully fixed yet. It does not work properly with array.map async fragments.

Here is the rendering diagram:

image

I could easily reproduce this in a generic astro application.

// AsyncComponent.astro

---
export interface Props {
  delay: number;
}

const {delay} = Astro.props as Props;

await new Promise(resolve => setTimeout(resolve, delay));
---
<div>{delay}</div>
// index.astro

---
import Layout from '../layouts/Layout.astro';
import Card from '../components/Card.astro';
import AsyncComponent from '../components/AsyncComponent.astro';

const asyncComponentDelays = [500, 1000, 1000, 2000, 1000];
---

<Layout title="Welcome to Astro.">
	<main>
		<h1>Welcome to <span class="text-gradient">Astro</span></h1>
		<p class="instructions">
			To get started, open the directory <code>src/pages</code> in your project.<br />
			<strong>Code Challenge:</strong> Tweak the "Welcome to Astro" message above.
		</p>
		
		{asyncComponentDelays.map((delay) => <AsyncComponent delay={delay} />)}

// ...

The result is 5.5s rendering (expected 2s):

image

If I add the following components

// index.astro
// ...
		<AsyncComponent delay={1000} />
		<AsyncComponent delay={1000} />
		<AsyncComponent delay={1000} />

		{asyncComponentDelays.map((delay) => <AsyncComponent delay={delay} />)}
// ...

Then it results in 6.5s rendering time, which also should not be the case (should remain 5.5s, of course if 5.5s were the right time):

image

Is there something to work this around?

Link to Minimal Reproducible Example

https://github.com/smnbbrv/astro-parallel-rendering-issue

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

The code for this is here (i think):

for (const value of child) {
yield markHTMLString(await renderChild(value));
}
for anyone looking to work on it.

@johannesspohr
Copy link
Contributor

@matthewp Thanks, I can have a look. I guess it makes sense to pull the parallelization logic out of the template rendering and make it reusable here.

@smnbbrv
Copy link
Author

smnbbrv commented Jun 5, 2023

Works perfectly, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants