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

Added preact-prepass-render to @astrojs/preact #7569

Closed

Conversation

aleksandrjet
Copy link
Contributor

Changes

Added preact-ssr-prepass library to preact integraiont. This library allow to use lazy components in preact code. Without this i get unknown error in SSR.

While execute SSR preact-ssr-prepass will wait to load all dynamic imports so that preact-render-to-string can use they. More details about preact-ssr-prepass

Testing

Added examples/lazy-preact and preact-lazy-component.test with lazy loading of component.

Docs

I don't think that it needs documentation, because i expected that Suspense and lazy functions will work.

It does not affect the current api, but it can slow build execution of preact components, because preact-ssr-prepass will add extra pass of tree. This pass will wait for all dynamic imports to be loaded, even if they don't exist in the code.

@aleksandrjet aleksandrjet requested a review from a team as a code owner July 5, 2023 06:41
@changeset-bot
Copy link

changeset-bot bot commented Jul 5, 2023

🦋 Changeset detected

Latest commit: 8059151

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 pkg: example Related to an example package (scope) pkg: preact Related to Preact (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Jul 5, 2023
@aleksandrjet aleksandrjet force-pushed the preact-prepass-render branch from 2000d0f to 2851c42 Compare July 5, 2023 08:54
@aleksandrjet aleksandrjet force-pushed the preact-prepass-render branch from 2851c42 to bd2394b Compare July 5, 2023 09:02
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that I don't think the Preact integration README needs updating for this, as it was never explicitly stated that you couldn't do this before, and we're just adding some functionality under the hood.

.changeset/big-pets-explode.md Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@aleksandrjet
Copy link
Contributor Author

do I need to do something else? I see that not all checks passed, but I can't restart them

@natemoo-re
Copy link
Member

do I need to do something else? I see that not all checks passed, but I can't restart them

No worries @aleksandrjet, we just have a couple of flaky CI tests that we're trying to track down! We're working through our backlog of PRs right now, hoping to give this a proper review soon!

@natemoo-re
Copy link
Member

Also I reached out to Marvin on Twitter about this one to check if preact-ssr-prepass is still the official recommendation from the Preact team. Hopefully it is!

https://twitter.com/n_moore/status/1678825870364930050

@natemoo-re natemoo-re linked an issue Aug 1, 2023 that may be closed by this pull request
1 task
@matthewp
Copy link
Contributor

@natemoo-re did you ever hear back about this?

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) pkg: example Related to an example package (scope) pkg: integration Related to any renderer integration (scope) pkg: preact Related to Preact (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Side effect of third argument of preact render
4 participants