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
Closed

fix(astrojs/cloudflare): SSR split file renaming misses ts endpoints #7555

wants to merge 12 commits into from

Conversation

alexanderniebuhr
Copy link
Member

@alexanderniebuhr alexanderniebuhr commented Jul 3, 2023

Changes

  • fixed an bug, where ts endpoint files are not renamed to .js after bundle step

Testing

  • updated test suite
  • manually

Docs

@changeset-bot
Copy link

changeset-bot bot commented Jul 3, 2023

🦋 Changeset detected

Latest commit: 45c39e5

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 the pkg: integration Related to any renderer integration (scope) label Jul 3, 2023
@alexanderniebuhr
Copy link
Member Author

cc @matthewp @ematipico

@alexanderniebuhr alexanderniebuhr requested a review from a team as a code owner July 3, 2023 18:20
@github-actions github-actions bot added feat: markdown Related to Markdown (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: example Related to an example package (scope) 🚨 action Modifies GitHub Actions pkg: react Related to React (scope) pkg: preact Related to Preact (scope) pkg: solid Related to Solid (scope) pkg: lit Related to Lit (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: astro Related to the core `astro` package (scope) and removed feat: markdown Related to Markdown (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: example Related to an example package (scope) 🚨 action Modifies GitHub Actions pkg: react Related to React (scope) pkg: preact Related to Preact (scope) pkg: solid Related to Solid (scope) pkg: lit Related to Lit (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: astro Related to the core `astro` package (scope) labels Jul 3, 2023
@alexanderniebuhr alexanderniebuhr changed the title fix bug, where ts files where not renamed correctly fix(astrojs/cloudflare): SSR split file rename logic Jul 3, 2023
@alexanderniebuhr alexanderniebuhr changed the title fix(astrojs/cloudflare): SSR split file rename logic fix(astrojs/cloudflare): SSR split file renaming misses ts endpoints Jul 3, 2023
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

Comment on lines +138 to 171
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);
}
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

fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
fix bug for cloudflare assets redirect
@alexanderniebuhr
Copy link
Member Author

had to redo the PR, due to git history conflicts: #7568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants