-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add private addPageExtension
hook
#3628
Conversation
|
// Otherwise, create a unique id for this set of hoisted scripts | ||
moduleId = `/astro/hoisted.js?q=${uniqueHoistedIds.size}`; | ||
uniqueHoistedIds.set(uniqueHoistedId, moduleId); | ||
if (metadata) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this noise is just moving anything that accesses metadata inside an if (metadata)
block. With custom pages, metadata
might not be defined.
if (!Component.isAstroComponentFactory) { | ||
const props: Record<string, any> = { ...(pageProps ?? {}), 'server:root': true }; | ||
const html = await renderComponent(result, Component.name, Component, props, null); | ||
page = { | ||
type: 'html', | ||
html: html.toString() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the module exports a non-astro file, we pass it through our component pipeline instead, which will use any provided renderers to render the component
// Semi-private `addPageExtension` hook | ||
Object.defineProperty(hooks, 'addPageExtension', { | ||
value: (...input: (string|string[])[]) => { | ||
const exts = (input.flat(Infinity) as string[]).map(ext => `.${ext.replace(/^\./, '')}`); | ||
updatedConfig._ctx.pageExtensions.push(...exts); | ||
}, | ||
writable: false, | ||
enumerable: false | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that private, but it works! It is intentionally not enumerable or exposed in the types, only there if you're looking for it.
if (isPage) { | ||
return html; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For hydration to work, we need to avoid stripping out astro-fragment
on the top-level page, so we inject a server:root
prop to track that.
8b8e61b
to
ee798ce
Compare
addPageExtensions
hookaddPageExtension
hook
@@ -57,46 +57,54 @@ export async function staticBuild(opts: StaticBuildOptions) { | |||
const [renderers, mod] = pageData.preload; | |||
const metadata = mod.$$metadata; | |||
|
|||
// Track client:only usage so we can map their CSS back to the Page they are used in. | |||
const clientOnlys = Array.from(metadata.clientOnlyComponentPaths()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's going to be merge conflicts after #3625 is merged. However all of this code goes away, so I think your changes won't be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, noticed this! Was glad to see this going away.
ee798ce
to
fcd240c
Compare
* feat: add private `addPageExtensions` hook * chore: remove renderer binding Co-authored-by: Nate Moore <nate@astro.build>
Changes
addPageExtension
hook to integrationsTesting
Test added
Docs
Not a public change