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

Fix Astro components parent-child render order #8187

Merged
merged 4 commits into from
Aug 23, 2023
Merged

Fix Astro components parent-child render order #8187

merged 4 commits into from
Aug 23, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Aug 22, 2023

Changes

Fix #8059

Previously, the renderComponent API will kick off the render directly, however I didn't notice that component slots are inited eagerly (for head propagation) and it kicked off the rendering of slots eagerly (not good).

This PR reverts to the previous technique where the parent will render each child in parallel manually instead. It renders all the child into a buffer first, then flush it out when it's its turn to render.

NOTE: While the benchmark shows that this is faster, it's because I removed the bufferChunks.length = 0 optimization which seems to do more harm than good for perf. If I ported that, the perf is actually slightly lower because things are less in parallel.

Testing

Added a new test

Docs

n/a. bug fix.

@changeset-bot
Copy link

changeset-bot bot commented Aug 22, 2023

🦋 Changeset detected

Latest commit: 26fa3e4

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Aug 22, 2023
@bluwy

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@bluwy
Copy link
Member Author

bluwy commented Aug 22, 2023

!bench server-stress

@github-actions
Copy link
Contributor

PR Benchmark

Server stress

Avg Stdev Max
Latency 2321.46 ms 619.93 ms 4433 ms
Avg Stdev Min Total in 30s
Req/Sec 417.07 156.28 291 12.5k requests
Bytes/Sec 29.6 MB 11.1 MB 20.7 MB 889 MB read

Main Benchmark

Server stress

Avg Stdev Max
Latency 2406.12 ms 423.84 ms 3707 ms
Avg Stdev Min Total in 30s
Req/Sec 400 177.6 370 12.0k requests
Bytes/Sec 28.4 MB 12.6 MB 26.3 MB 853 MB read

@bluwy bluwy marked this pull request as ready for review August 22, 2023 14:05
@bluwy bluwy merged commit 273335c into main Aug 23, 2023
13 checks passed
@bluwy bluwy deleted the fix-render-order branch August 23, 2023 12:29
@astrobot-houston astrobot-houston mentioned this pull request Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In different astro versions, the execution order of parent and child components is inconsistent
3 participants