From f8b21eb4c3ead2d90944db5792f907a375bceeec Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 21 Nov 2023 15:40:59 -0600 Subject: [PATCH] =?UTF-8?q?Revert=20"fix(i18n):=20fallback=20should=20crea?= =?UTF-8?q?te=20consistent=20directories=20(#9142=E2=80=A6=20(#9157)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/olive-socks-double.md | 5 + packages/astro/src/core/build/generate.ts | 403 +++++++++--------- .../astro/src/core/routing/manifest/create.ts | 23 +- packages/astro/src/core/routing/params.ts | 2 +- packages/astro/src/i18n/middleware.ts | 4 +- packages/astro/test/i18n-routing.test.js | 3 +- .../astro/test/ssr-split-manifest.test.js | 1 + 7 files changed, 228 insertions(+), 213 deletions(-) create mode 100644 .changeset/olive-socks-double.md diff --git a/.changeset/olive-socks-double.md b/.changeset/olive-socks-double.md new file mode 100644 index 0000000000000..e0cdfc794b7f0 --- /dev/null +++ b/.changeset/olive-socks-double.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Revert fix around fallback system, which broken injected styles diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 1ac2f05b690e9..35f8ecb6673ec 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -13,7 +13,6 @@ import type { MiddlewareEndpointHandler, RouteData, RouteType, - SSRElement, SSRError, SSRLoadedRenderer, SSRManifest, @@ -262,49 +261,21 @@ async function generatePage( builtPaths: Set, pipeline: BuildPipeline ) { - // prepare information we need + let timeStart = performance.now(); const logger = pipeline.getLogger(); const config = pipeline.getConfig(); - const manifest = pipeline.getManifest(); - const pageModulePromise = ssrEntry.page; - const onRequest = ssrEntry.onRequest; const pageInfo = getPageDataByComponent(pipeline.getInternals(), pageData.route.component); - // Calculate information of the page, like scripts, links and styles - const hoistedScripts = pageInfo?.hoistedScript ?? null; - const moduleStyles = pageData.styles + // may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc. + const linkIds: [] = []; + const scripts = pageInfo?.hoistedScript ?? null; + const styles = pageData.styles .sort(cssOrder) .map(({ sheet }) => sheet) .reduce(mergeInlineCss, []); - // may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc. - const links = new Set(); - const styles = createStylesheetElementSet(moduleStyles, manifest.base, manifest.assetsPrefix); - const scripts = createModuleScriptsSet( - hoistedScripts ? [hoistedScripts] : [], - manifest.base, - manifest.assetsPrefix - ); - if (pipeline.getSettings().scripts.some((script) => script.stage === 'page')) { - const hashedFilePath = pipeline.getInternals().entrySpecifierToBundleMap.get(PAGE_SCRIPT_ID); - if (typeof hashedFilePath !== 'string') { - throw new Error(`Cannot find the built path for ${PAGE_SCRIPT_ID}`); - } - const src = createAssetLink(hashedFilePath, manifest.base, manifest.assetsPrefix); - scripts.add({ - props: { type: 'module', src }, - children: '', - }); - } - // Add all injected scripts to the page. - for (const script of pipeline.getSettings().scripts) { - if (script.stage === 'head-inline') { - scripts.add({ - props: {}, - children: script.content, - }); - } - } - // prepare the middleware + + const pageModulePromise = ssrEntry.page; + const onRequest = ssrEntry.onRequest; const i18nMiddleware = createI18nMiddleware( pipeline.getManifest().i18n, pipeline.getManifest().base, @@ -338,24 +309,43 @@ async function generatePage( return; } - // Now we explode the routes. A route render itself, and it can render its fallbacks (i18n routing) - for (const route of eachRouteInRouteData(pageData)) { - // Get paths for the route, calling getStaticPaths if needed. - const paths = await getPathsForRoute(route, pageModule, pipeline, builtPaths); - let timeStart = performance.now(); - let prevTimeEnd = timeStart; - for (let i = 0; i < paths.length; i++) { - const path = paths[i]; - pipeline.getEnvironment().logger.debug('build', `Generating: ${path}`); - await generatePath(path, pipeline, route, links, scripts, styles, pageModule); - const timeEnd = performance.now(); - const timeChange = getTimeStat(prevTimeEnd, timeEnd); - const timeIncrease = `(+${timeChange})`; - const filePath = getOutputFilename(pipeline.getConfig(), path, pageData.route.type); - const lineIcon = i === paths.length - 1 ? '└─' : '├─'; - logger.info(null, ` ${cyan(lineIcon)} ${dim(filePath)} ${dim(timeIncrease)}`); - prevTimeEnd = timeEnd; + const generationOptions: Readonly = { + pageData, + linkIds, + scripts, + styles, + mod: pageModule, + }; + + const icon = + pageData.route.type === 'page' || + pageData.route.type === 'redirect' || + pageData.route.type === 'fallback' + ? green('▶') + : magenta('λ'); + if (isRelativePath(pageData.route.component)) { + logger.info(null, `${icon} ${pageData.route.route}`); + for (const fallbackRoute of pageData.route.fallbackRoutes) { + logger.info(null, `${icon} ${fallbackRoute.route}`); } + } else { + logger.info(null, `${icon} ${pageData.route.component}`); + } + + // Get paths for the route, calling getStaticPaths if needed. + const paths = await getPathsForRoute(pageData, pageModule, pipeline, builtPaths); + + let prevTimeEnd = timeStart; + for (let i = 0; i < paths.length; i++) { + const path = paths[i]; + await generatePath(path, generationOptions, pipeline); + const timeEnd = performance.now(); + const timeChange = getTimeStat(prevTimeEnd, timeEnd); + const timeIncrease = `(+${timeChange})`; + const filePath = getOutputFilename(pipeline.getConfig(), path, pageData.route.type); + const lineIcon = i === paths.length - 1 ? '└─' : '├─'; + logger.info(null, ` ${cyan(lineIcon)} ${dim(filePath)} ${dim(timeIncrease)}`); + prevTimeEnd = timeEnd; } } @@ -367,7 +357,7 @@ function* eachRouteInRouteData(data: PageBuildData) { } async function getPathsForRoute( - route: RouteData, + pageData: PageBuildData, mod: ComponentInstance, pipeline: BuildPipeline, builtPaths: Set @@ -375,64 +365,68 @@ async function getPathsForRoute( const opts = pipeline.getStaticBuildOptions(); const logger = pipeline.getLogger(); let paths: Array = []; - if (route.pathname) { - paths.push(route.pathname); - builtPaths.add(route.pathname); - for (const virtualRoute of route.fallbackRoutes) { + if (pageData.route.pathname) { + paths.push(pageData.route.pathname); + builtPaths.add(pageData.route.pathname); + for (const virtualRoute of pageData.route.fallbackRoutes) { if (virtualRoute.pathname) { paths.push(virtualRoute.pathname); builtPaths.add(virtualRoute.pathname); } } } else { - const staticPaths = await callGetStaticPaths({ - mod, - route, - routeCache: opts.routeCache, - logger, - ssr: isServerLikeOutput(opts.settings.config), - }).catch((err) => { - logger.debug('build', `├── ${colors.bold(colors.red('✗'))} ${route.component}`); - throw err; - }); - - const label = staticPaths.length === 1 ? 'page' : 'pages'; - logger.debug( - 'build', - `├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.magenta( - `[${staticPaths.length} ${label}]` - )}` - ); + for (const route of eachRouteInRouteData(pageData)) { + const staticPaths = await callGetStaticPaths({ + mod, + route, + routeCache: opts.routeCache, + logger, + ssr: isServerLikeOutput(opts.settings.config), + }).catch((err) => { + logger.debug('build', `├── ${colors.bold(colors.red('✗'))} ${route.component}`); + throw err; + }); - paths = staticPaths - .map((staticPath) => { - try { - return route.generate(staticPath.params); - } catch (e) { - if (e instanceof TypeError) { - throw getInvalidRouteSegmentError(e, route, staticPath); - } - throw e; - } - }) - .filter((staticPath) => { - // The path hasn't been built yet, include it - if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) { - return true; - } + const label = staticPaths.length === 1 ? 'page' : 'pages'; + logger.debug( + 'build', + `├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.magenta( + `[${staticPaths.length} ${label}]` + )}` + ); - // The path was already built once. Check the manifest to see if - // this route takes priority for the final URL. - // NOTE: The same URL may match multiple routes in the manifest. - // Routing priority needs to be verified here for any duplicate - // paths to ensure routing priority rules are enforced in the final build. - const matchedRoute = matchRoute(staticPath, opts.manifest); - return matchedRoute === route; - }); + paths.push( + ...staticPaths + .map((staticPath) => { + try { + return route.generate(staticPath.params); + } catch (e) { + if (e instanceof TypeError) { + throw getInvalidRouteSegmentError(e, route, staticPath); + } + throw e; + } + }) + .filter((staticPath) => { + // The path hasn't been built yet, include it + if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) { + return true; + } + + // The path was already built once. Check the manifest to see if + // this route takes priority for the final URL. + // NOTE: The same URL may match multiple routes in the manifest. + // Routing priority needs to be verified here for any duplicate + // paths to ensure routing priority rules are enforced in the final build. + const matchedRoute = matchRoute(staticPath, opts.manifest); + return matchedRoute === route; + }) + ); - // Add each path to the builtPaths set, to avoid building it again later. - for (const staticPath of paths) { - builtPaths.add(removeTrailingForwardSlash(staticPath)); + // Add each path to the builtPaths set, to avoid building it again later. + for (const staticPath of paths) { + builtPaths.add(removeTrailingForwardSlash(staticPath)); + } } } @@ -515,92 +509,106 @@ function getUrlForPath( return url; } -async function generatePath( - pathname: string, - pipeline: BuildPipeline, - route: RouteData, - links: Set, - scripts: Set, - styles: Set, - mod: ComponentInstance -) { +async function generatePath(pathname: string, gopts: GeneratePathOptions, pipeline: BuildPipeline) { const manifest = pipeline.getManifest(); - const logger = pipeline.getLogger(); - pipeline.getEnvironment().logger.debug('build', `Generating: ${pathname}`); + const { mod, scripts: hoistedScripts, styles: _styles, pageData } = gopts; - const icon = - route.type === 'page' || route.type === 'redirect' || route.type === 'fallback' - ? green('▶') - : magenta('λ'); - if (isRelativePath(route.component)) { - logger.info(null, `${icon} ${route.route}`); - } else { - logger.info(null, `${icon} ${route.component}`); - } + for (const route of eachRouteInRouteData(pageData)) { + // This adds the page name to the array so it can be shown as part of stats. + if (route.type === 'page') { + addPageName(pathname, pipeline.getStaticBuildOptions()); + } - // This adds the page name to the array so it can be shown as part of stats. - if (route.type === 'page') { - addPageName(pathname, pipeline.getStaticBuildOptions()); - } + pipeline.getEnvironment().logger.debug('build', `Generating: ${pathname}`); - const ssr = isServerLikeOutput(pipeline.getConfig()); - const url = getUrlForPath( - pathname, - pipeline.getConfig().base, - pipeline.getStaticBuildOptions().origin, - pipeline.getConfig().build.format, - route.type - ); + // may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc. + const links = new Set(); + const scripts = createModuleScriptsSet( + hoistedScripts ? [hoistedScripts] : [], + manifest.base, + manifest.assetsPrefix + ); + const styles = createStylesheetElementSet(_styles, manifest.base, manifest.assetsPrefix); - const request = createRequest({ - url, - headers: new Headers(), - logger: pipeline.getLogger(), - ssr, - }); - const i18n = pipeline.getConfig().experimental.i18n; + if (pipeline.getSettings().scripts.some((script) => script.stage === 'page')) { + const hashedFilePath = pipeline.getInternals().entrySpecifierToBundleMap.get(PAGE_SCRIPT_ID); + if (typeof hashedFilePath !== 'string') { + throw new Error(`Cannot find the built path for ${PAGE_SCRIPT_ID}`); + } + const src = createAssetLink(hashedFilePath, manifest.base, manifest.assetsPrefix); + scripts.add({ + props: { type: 'module', src }, + children: '', + }); + } - const renderContext = await createRenderContext({ - pathname, - request, - componentMetadata: manifest.componentMetadata, - scripts, - styles, - links, - route, - env: pipeline.getEnvironment(), - mod, - locales: i18n?.locales, - routingStrategy: i18n?.routingStrategy, - defaultLocale: i18n?.defaultLocale, - }); + // Add all injected scripts to the page. + for (const script of pipeline.getSettings().scripts) { + if (script.stage === 'head-inline') { + scripts.add({ + props: {}, + children: script.content, + }); + } + } - let body: string | Uint8Array; - let encoding: BufferEncoding | undefined; + const ssr = isServerLikeOutput(pipeline.getConfig()); + const url = getUrlForPath( + pathname, + pipeline.getConfig().base, + pipeline.getStaticBuildOptions().origin, + pipeline.getConfig().build.format, + route.type + ); - let response: Response; - try { - response = await pipeline.renderRoute(renderContext, mod); - } catch (err) { - if (!AstroError.is(err) && !(err as SSRError).id && typeof err === 'object') { - (err as SSRError).id = route.component; - } - throw err; - } + const request = createRequest({ + url, + headers: new Headers(), + logger: pipeline.getLogger(), + ssr, + }); + const i18n = pipeline.getConfig().experimental.i18n; + const renderContext = await createRenderContext({ + pathname, + request, + componentMetadata: manifest.componentMetadata, + scripts, + styles, + links, + route, + env: pipeline.getEnvironment(), + mod, + locales: i18n?.locales, + routingStrategy: i18n?.routingStrategy, + defaultLocale: i18n?.defaultLocale, + }); + + let body: string | Uint8Array; + let encoding: BufferEncoding | undefined; - if (response.status >= 300 && response.status < 400) { - // If redirects is set to false, don't output the HTML - if (!pipeline.getConfig().build.redirects) { - return; + let response: Response; + try { + response = await pipeline.renderRoute(renderContext, mod); + } catch (err) { + if (!AstroError.is(err) && !(err as SSRError).id && typeof err === 'object') { + (err as SSRError).id = pageData.component; + } + throw err; } - const locationSite = getRedirectLocationOrThrow(response.headers); - const siteURL = pipeline.getConfig().site; - const location = siteURL ? new URL(locationSite, siteURL) : locationSite; - const fromPath = new URL(renderContext.request.url).pathname; - // A short delay causes Google to interpret the redirect as temporary. - // https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh - const delay = response.status === 302 ? 2 : 0; - body = ` + + if (response.status >= 300 && response.status < 400) { + // If redirects is set to false, don't output the HTML + if (!pipeline.getConfig().build.redirects) { + return; + } + const locationSite = getRedirectLocationOrThrow(response.headers); + const siteURL = pipeline.getConfig().site; + const location = siteURL ? new URL(locationSite, siteURL) : locationSite; + const fromPath = new URL(renderContext.request.url).pathname; + // A short delay causes Google to interpret the redirect as temporary. + // https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh + const delay = response.status === 302 ? 2 : 0; + body = ` Redirecting to: ${location} @@ -608,26 +616,27 @@ async function generatePath( Redirecting from ${fromPath} to ${location} `; - if (pipeline.getConfig().compressHTML === true) { - body = body.replaceAll('\n', ''); - } - // A dynamic redirect, set the location so that integrations know about it. - if (route.type !== 'redirect') { - route.redirect = location.toString(); + if (pipeline.getConfig().compressHTML === true) { + body = body.replaceAll('\n', ''); + } + // A dynamic redirect, set the location so that integrations know about it. + if (route.type !== 'redirect') { + route.redirect = location.toString(); + } + } else { + // If there's no body, do nothing + if (!response.body) return; + body = Buffer.from(await response.arrayBuffer()); + encoding = (response.headers.get('X-Astro-Encoding') as BufferEncoding | null) ?? 'utf-8'; } - } else { - // If there's no body, do nothing - if (!response.body) return; - body = Buffer.from(await response.arrayBuffer()); - encoding = (response.headers.get('X-Astro-Encoding') as BufferEncoding | null) ?? 'utf-8'; - } - const outFolder = getOutFolder(pipeline.getConfig(), pathname, route.type); - const outFile = getOutFile(pipeline.getConfig(), outFolder, pathname, route.type); - route.distURL = outFile; + const outFolder = getOutFolder(pipeline.getConfig(), pathname, route.type); + const outFile = getOutFile(pipeline.getConfig(), outFolder, pathname, route.type); + route.distURL = outFile; - await fs.promises.mkdir(outFolder, { recursive: true }); - await fs.promises.writeFile(outFile, body, encoding); + await fs.promises.mkdir(outFolder, { recursive: true }); + await fs.promises.writeFile(outFile, body, encoding); + } } /** diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index b6960e3da3d2e..44482fdcbbc93 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -603,22 +603,22 @@ export function createRouteManifest( if (!hasRoute) { let pathname: string | undefined; let route: string; - if ( - fallbackToLocale === i18n.defaultLocale && - i18n.routingStrategy === 'prefix-other-locales' - ) { + if (fallbackToLocale === i18n.defaultLocale) { if (fallbackToRoute.pathname) { pathname = `/${fallbackFromLocale}${fallbackToRoute.pathname}`; } route = `/${fallbackFromLocale}${fallbackToRoute.route}`; } else { - pathname = fallbackToRoute.pathname - ?.replace(`/${fallbackToLocale}/`, `/${fallbackFromLocale}/`) - .replace(`/${fallbackToLocale}`, `/${fallbackFromLocale}`); - route = fallbackToRoute.route - .replace(`/${fallbackToLocale}`, `/${fallbackFromLocale}`) - .replace(`/${fallbackToLocale}/`, `/${fallbackFromLocale}/`); + pathname = fallbackToRoute.pathname?.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` + ); + route = fallbackToRoute.route.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` + ); } + const segments = removeLeadingForwardSlash(route) .split(path.posix.sep) .filter(Boolean) @@ -626,7 +626,7 @@ export function createRouteManifest( validateSegment(s); return getParts(s, route); }); - const generate = getRouteGenerator(segments, config.trailingSlash); + const index = routes.findIndex((r) => r === fallbackToRoute); if (index) { const fallbackRoute: RouteData = { @@ -634,7 +634,6 @@ export function createRouteManifest( pathname, route, segments, - generate, pattern: getPattern(segments, config, config.trailingSlash), type: 'fallback', fallbackRoutes: [], diff --git a/packages/astro/src/core/routing/params.ts b/packages/astro/src/core/routing/params.ts index 56497dac641c3..987528d5786aa 100644 --- a/packages/astro/src/core/routing/params.ts +++ b/packages/astro/src/core/routing/params.ts @@ -31,7 +31,7 @@ export function getParams(array: string[]) { export function stringifyParams(params: GetStaticPathsItem['params'], route: RouteData) { // validate parameter values then stringify each value const validatedParams = Object.entries(params).reduce((acc, next) => { - validateGetStaticPathsParameter(next, route.route); + validateGetStaticPathsParameter(next, route.component); const [key, value] = next; if (value !== undefined) { acc[key] = typeof value === 'string' ? trimSlashes(value) : value.toString(); diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index 03b7e40179e11..854a39b77cda3 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -41,7 +41,7 @@ export function createI18nMiddleware( } const url = context.url; - const { locales, defaultLocale, fallback, routingStrategy } = i18n; + const { locales, defaultLocale, fallback } = i18n; const response = await next(); if (response instanceof Response) { @@ -82,7 +82,7 @@ export function createI18nMiddleware( let newPathname: string; // If a locale falls back to the default locale, we want to **remove** the locale because // the default locale doesn't have a prefix - if (fallbackLocale === defaultLocale && routingStrategy === 'prefix-other-locales') { + if (fallbackLocale === defaultLocale) { newPathname = url.pathname.replace(`/${urlLocale}`, ``); } else { newPathname = url.pathname.replace(`/${urlLocale}`, `/${fallbackLocale}`); diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js index 4ec355a81c521..2c9b878138b8a 100644 --- a/packages/astro/test/i18n-routing.test.js +++ b/packages/astro/test/i18n-routing.test.js @@ -668,7 +668,8 @@ describe('[SSG] i18n routing', () => { await fixture.build(); }); - it('should render the en locale', async () => { + // TODO: enable once we fix fallback + it.skip('should render the en locale', async () => { let html = await fixture.readFile('/it/start/index.html'); expect(html).to.include('http-equiv="refresh'); expect(html).to.include('url=/new-site/en/start'); diff --git a/packages/astro/test/ssr-split-manifest.test.js b/packages/astro/test/ssr-split-manifest.test.js index 74d2fe74e5747..89c8e00ef8980 100644 --- a/packages/astro/test/ssr-split-manifest.test.js +++ b/packages/astro/test/ssr-split-manifest.test.js @@ -109,6 +109,7 @@ describe('astro:ssr-manifest, split', () => { const request = new Request('http://example.com/'); const response = await app.render(request); const html = await response.text(); + console.log(html); expect(html.includes('Testing')).to.be.true; }); });