-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()); | ||
trackClientOnlyPageDatas(internals, pageData, clientOnlys); | ||
|
||
const topLevelImports = new Set([ | ||
// Any component that gets hydrated | ||
// 'components/Counter.jsx' | ||
// { 'components/Counter.jsx': 'counter.hash.js' } | ||
...metadata.hydratedComponentPaths(), | ||
// Client-only components | ||
...clientOnlys, | ||
// The client path for each renderer | ||
...renderers | ||
.filter((renderer) => !!renderer.clientEntrypoint) | ||
.map((renderer) => renderer.clientEntrypoint!), | ||
]); | ||
|
||
// Add hoisted scripts | ||
const hoistedScripts = new Set(metadata.hoistedScriptPaths()); | ||
if (hoistedScripts.size) { | ||
const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort()); | ||
let moduleId: string; | ||
|
||
// If we're already tracking this set of hoisted scripts, get the unique id | ||
if (uniqueHoistedIds.has(uniqueHoistedId)) { | ||
moduleId = uniqueHoistedIds.get(uniqueHoistedId)!; | ||
} else { | ||
// 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 commentThe reason will be displayed to describe this comment to others. Learn more. All this noise is just moving anything that accesses metadata inside an |
||
// Any component that gets hydrated | ||
// 'components/Counter.jsx' | ||
// { 'components/Counter.jsx': 'counter.hash.js' } | ||
for (const hydratedComponentPath of metadata.hydratedComponentPaths()) { | ||
topLevelImports.add(hydratedComponentPath); | ||
} | ||
|
||
// Track client:only usage so we can map their CSS back to the Page they are used in. | ||
const clientOnlys = Array.from(metadata.clientOnlyComponentPaths()); | ||
trackClientOnlyPageDatas(internals, pageData, clientOnlys); | ||
|
||
// Client-only components | ||
for (const clientOnly of clientOnlys) { | ||
topLevelImports.add(clientOnly) | ||
} | ||
topLevelImports.add(moduleId); | ||
|
||
// Make sure to track that this page uses this set of hoisted scripts | ||
if (internals.hoistedScriptIdToPagesMap.has(moduleId)) { | ||
const pages = internals.hoistedScriptIdToPagesMap.get(moduleId); | ||
pages!.add(astroModuleId); | ||
} else { | ||
internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId])); | ||
internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts); | ||
|
||
// Add hoisted scripts | ||
const hoistedScripts = new Set(metadata.hoistedScriptPaths()); | ||
if (hoistedScripts.size) { | ||
const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort()); | ||
let moduleId: string; | ||
|
||
// If we're already tracking this set of hoisted scripts, get the unique id | ||
if (uniqueHoistedIds.has(uniqueHoistedId)) { | ||
moduleId = uniqueHoistedIds.get(uniqueHoistedId)!; | ||
} else { | ||
// Otherwise, create a unique id for this set of hoisted scripts | ||
moduleId = `/astro/hoisted.js?q=${uniqueHoistedIds.size}`; | ||
uniqueHoistedIds.set(uniqueHoistedId, moduleId); | ||
} | ||
topLevelImports.add(moduleId); | ||
|
||
// Make sure to track that this page uses this set of hoisted scripts | ||
if (internals.hoistedScriptIdToPagesMap.has(moduleId)) { | ||
const pages = internals.hoistedScriptIdToPagesMap.get(moduleId); | ||
pages!.add(astroModuleId); | ||
} else { | ||
internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId])); | ||
internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ import type { | |
} from '../../@types/astro'; | ||
import type { LogOptions } from '../logger/core.js'; | ||
|
||
import { renderHead, renderPage } from '../../runtime/server/index.js'; | ||
import { renderHead, renderPage, renderComponent } from '../../runtime/server/index.js'; | ||
import { getParams } from '../routing/params.js'; | ||
import { createResult } from './result.js'; | ||
import { callGetStaticPaths, findPathItemByKey, RouteCache } from './route-cache.js'; | ||
|
@@ -126,8 +126,6 @@ export async function render( | |
const Component = await mod.default; | ||
if (!Component) | ||
throw new Error(`Expected an exported Astro component but received typeof ${typeof Component}`); | ||
if (!Component.isAstroComponentFactory) | ||
throw new Error(`Unable to SSR non-Astro component (${route?.component})`); | ||
|
||
const result = createResult({ | ||
links, | ||
|
@@ -146,7 +144,17 @@ export async function render( | |
ssr, | ||
}); | ||
|
||
let page = await renderPage(result, Component, pageProps, null); | ||
let page: Awaited<ReturnType<typeof renderPage>>; | ||
if (!Component.isAstroComponentFactory) { | ||
const props: Record<string, any> = { ...(pageProps ?? {}), 'server:root': true }; | ||
natemoo-re marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const html = await renderComponent(result, Component.name, Component, props, null); | ||
page = { | ||
type: 'html', | ||
html: html.toString() | ||
} | ||
Comment on lines
+148
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} else { | ||
page = await renderPage(result, Component, pageProps, null); | ||
} | ||
|
||
if (page.type === 'response') { | ||
return page; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,15 @@ | ||
import type { AddressInfo } from 'net'; | ||
import type { ViteDevServer } from 'vite'; | ||
import { AstroConfig, AstroRenderer, BuildConfig, RouteData } from '../@types/astro.js'; | ||
import { AstroConfig, AstroIntegration, AstroRenderer, BuildConfig, RouteData } from '../@types/astro.js'; | ||
import ssgAdapter from '../adapter-ssg/index.js'; | ||
import type { SerializedSSRManifest } from '../core/app/types'; | ||
import type { PageBuildData } from '../core/build/types'; | ||
import { mergeConfig } from '../core/config.js'; | ||
import type { ViteConfigWithSSR } from '../core/create-vite.js'; | ||
import { isBuildingToSSR } from '../core/util.js'; | ||
|
||
type Hooks<Hook extends keyof AstroIntegration['hooks'], Fn = AstroIntegration['hooks'][Hook]> = Fn extends (...args: any) => any ? Parameters<Fn>[0] : never; | ||
|
||
export async function runHookConfigSetup({ | ||
config: _config, | ||
command, | ||
|
@@ -34,7 +36,7 @@ export async function runHookConfigSetup({ | |
* ``` | ||
*/ | ||
if (integration?.hooks?.['astro:config:setup']) { | ||
await integration.hooks['astro:config:setup']({ | ||
const hooks: Hooks<'astro:config:setup'> = { | ||
config: updatedConfig, | ||
command, | ||
addRenderer(renderer: AstroRenderer) { | ||
|
@@ -49,7 +51,17 @@ export async function runHookConfigSetup({ | |
injectRoute: (injectRoute) => { | ||
updatedConfig._ctx.injectedRoutes.push(injectRoute); | ||
}, | ||
}); | ||
} | ||
// 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 | ||
}) | ||
Comment on lines
+55
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
await integration.hooks['astro:config:setup'](hooks); | ||
} | ||
} | ||
return updatedConfig; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,7 +181,7 @@ export async function renderComponent( | |
const { renderers } = result._metadata; | ||
const metadata: AstroComponentMetadata = { displayName }; | ||
|
||
const { hydration, props } = extractDirectives(_props); | ||
const { hydration, isPage, props } = extractDirectives(_props); | ||
let html = ''; | ||
let needsHydrationScript = hydration && determineIfNeedsHydrationScript(result); | ||
let needsDirectiveScript = | ||
|
@@ -317,6 +317,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr | |
} | ||
|
||
if (!hydration) { | ||
if (isPage) { | ||
return html; | ||
} | ||
Comment on lines
+320
to
+322
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For hydration to work, we need to avoid stripping out |
||
return markHTMLString(html.replace(/\<\/?astro-fragment\>/g, '')); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { defineConfig } from 'rollup' | ||
import test from './integration.js' | ||
|
||
export default defineConfig({ | ||
integrations: [test()] | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
export default function() { | ||
return { | ||
name: '@astrojs/test-integration', | ||
hooks: { | ||
'astro:config:setup': ({ addPageExtension }) => { | ||
addPageExtension('.mjs') | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"name": "@test/integration-add-page-extension", | ||
"type": "module", | ||
"version": "0.0.0", | ||
"private": true, | ||
"dependencies": { | ||
"astro": "workspace:*" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<h1>Hello world!</h1> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// Convulted test case, rexport astro file from new `.mjs` page | ||
import Test from '../components/test.astro'; | ||
export default Test; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { expect } from 'chai'; | ||
import * as cheerio from 'cheerio'; | ||
import { loadFixture } from './test-utils.js'; | ||
|
||
describe('Integration addPageExtension', () => { | ||
/** @type {import('./test-utils').Fixture} */ | ||
let fixture; | ||
|
||
before(async () => { | ||
fixture = await loadFixture({ root: './fixtures/integration-add-page-extension/' }); | ||
await fixture.build(); | ||
}); | ||
|
||
it('supports .mjs files', async () => { | ||
const html = await fixture.readFile('/test/index.html'); | ||
const $ = cheerio.load(html); | ||
expect($('h1').text()).to.equal('Hello world!'); | ||
}); | ||
}); |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.