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(astrojs/cloudflare): SSR split file renaming misses ts endpoints #7555

Closed
wants to merge 12 commits into from
5 changes: 5 additions & 0 deletions .changeset/shiny-parrots-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/cloudflare': patch
---

fix bug where asset redirects caused Cloudflare error
5 changes: 5 additions & 0 deletions .changeset/sweet-bats-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/cloudflare': patch
---

fix bug where `.ts` files are not renamed to `.js`
86 changes: 53 additions & 33 deletions packages/integrations/cloudflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { AstroAdapter, AstroConfig, AstroIntegration, RouteData } from 'ast
import esbuild from 'esbuild';
import * as fs from 'fs';
import * as os from 'os';
import { dirname } from 'path';
import { sep } from 'path';
import glob from 'tiny-glob';
import { fileURLToPath, pathToFileURL } from 'url';

Expand All @@ -21,15 +21,15 @@ interface BuildConfig {
export function getAdapter(isModeDirectory: boolean): AstroAdapter {
return isModeDirectory
? {
name: '@astrojs/cloudflare',
serverEntrypoint: '@astrojs/cloudflare/server.directory.js',
exports: ['onRequest', 'manifest'],
}
name: '@astrojs/cloudflare',
serverEntrypoint: '@astrojs/cloudflare/server.directory.js',
exports: ['onRequest', 'manifest'],
}
: {
name: '@astrojs/cloudflare',
serverEntrypoint: '@astrojs/cloudflare/server.advanced.js',
exports: ['default'],
};
name: '@astrojs/cloudflare',
serverEntrypoint: '@astrojs/cloudflare/server.advanced.js',
exports: ['default'],
};
}

const SHIM = `globalThis.process = {
Expand Down Expand Up @@ -104,13 +104,12 @@ export default function createIntegration(args?: Options): AstroIntegration {
}

if (isModeDirectory && _buildConfig.split) {
const entryPointsRouteData = [..._entryPoints.keys()];
const entryPointsURL = [..._entryPoints.values()];
const entryPaths = entryPointsURL.map((entry) => fileURLToPath(entry));
const outputDir = fileURLToPath(new URL('.astro', _buildConfig.server));
const outputUrl = new URL('$astro', _buildConfig.server)
const outputDir = fileURLToPath(outputUrl);

// NOTE: AFAIK, esbuild keeps the order of the entryPoints array
const { outputFiles } = await esbuild.build({
await esbuild.build({
target: 'es2020',
platform: 'browser',
conditions: ['workerd', 'worker', 'browser'],
Expand All @@ -126,28 +125,49 @@ export default function createIntegration(args?: Options): AstroIntegration {
logOverride: {
'ignored-bare-import': 'silent',
},
write: false,
});

// loop through all bundled files and write them to the functions folder
for (const [index, outputFile] of outputFiles.entries()) {
// we need to make sure the filename in the functions folder
// matches to cloudflares routing capabilities (see their docs)
// IN: src/pages/[language]/files/[...path].astro
// OUT: [language]/files/[[path]].js
const fileName = entryPointsRouteData[index].component
.replace('src/pages/', '')
.replace('.astro', '.js')
.replace(/(\[\.\.\.)(\w+)(\])/g, (_match, _p1, p2) => {
return `[[${p2}]]`;
});

const fileUrl = new URL(fileName, functionsUrl);
const newFileDir = dirname(fileURLToPath(fileUrl));
if (!fs.existsSync(newFileDir)) {
fs.mkdirSync(newFileDir, { recursive: true });
}
await fs.promises.writeFile(fileUrl, outputFile.contents);
const outputFiles: Array<string> = (
await glob(`**/*`, {
cwd: outputDir,
filesOnly: true,
})
)

// loop through all new bundled files and write them to the functions folder
for (const outputFile of outputFiles) {

// split the path into an array
const path = outputFile.split(sep);

// replace dynamic path with [path]
const pathWithDynamics = path.map((segment) => segment.replace(/(\_)(\w+)(\_)/g, (_, __, prop) => {
return `[${prop}]`;
}));

// replace nested dynamic path with [[path]]
const pathWithNestedDynamics = pathWithDynamics.map((segment) => segment.replace(/(\_\-\-\-)(\w+)(\_)/g, (_, __, prop) => {
return `[[${prop}]]`;
}))

// remove original file extension
const pathReversed = pathWithNestedDynamics.reverse();
pathReversed[0] = pathReversed[0]
.replace('entry.', '')
.replace(/(.*)\.(\w+)\.(\w+)$/g, (_, fileName, oldExt, newExt) => {
return `${fileName}.${newExt}`;
})

const finalSegments = pathReversed.reverse();
const finalDirPath = finalSegments.slice(0, -1).join('/');
const finalPath = finalSegments.join('/');

const newDirUrl = new URL(finalDirPath, functionsUrl);
await fs.promises.mkdir(newDirUrl, { recursive: true })

const oldFileUrl = new URL(`$astro/${outputFile}`, outputUrl);
const newFileUrl = new URL(finalPath, functionsUrl);
await fs.promises.rename(oldFileUrl, newFileUrl);
}
Comment on lines +138 to 171
Copy link
Member Author

Choose a reason for hiding this comment

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

This loop has a lot of memory allocation, let me know if I should consolidate the steps into more abstract steps?
@ematipico @matthewp

} else {
const entryPath = fileURLToPath(new URL(_buildConfig.serverEntry, _buildConfig.server));
Expand Down
8 changes: 7 additions & 1 deletion packages/integrations/cloudflare/src/server.directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ export function createExports(manifest: SSRManifest) {
const { pathname } = new URL(request.url);
// static assets fallback, in case default _routes.json is not used
if (manifest.assets.has(pathname)) {
return next(request);
// we need this so the page does not error
// https://developers.cloudflare.com/pages/platform/functions/advanced-mode/#set-up-a-function
return (runtimeEnv.env as unknown & {
ASSETS: {
fetch: typeof fetch;
};
Copy link
Member Author

Choose a reason for hiding this comment

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

this is already done for advanced mode but was missing for directroy mode. please also note that I'll remove the type assertion on the Astro.locals PR, where I refactored the EventContext typing

}).ASSETS.fetch(request);
}

let routeData = app.match(request, { matchNotFound: true });
Expand Down
3 changes: 3 additions & 0 deletions packages/integrations/cloudflare/test/directory-split.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ describe('Cloudflare SSR split', () => {
expect(await fixture.pathExists('../functions/[person]/[car].js')).to.be.true;
expect(await fixture.pathExists('../functions/files/[[path]].js')).to.be.true;
expect(await fixture.pathExists('../functions/[language]/files/[[path]].js')).to.be.true;
expect(await fixture.pathExists('../functions/trpc/[trpc].js')).to.be.true;
expect(await fixture.pathExists('../functions/javascript.js')).to.be.true;
expect(await fixture.pathExists('../functions/test.json.js')).to.be.true;
});

it('generates pre-rendered files', async () => {
Expand Down