-
-
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
fix(actions): support trailing slash #12657
Conversation
🦋 Changeset detectedLatest commit: 35ba7e5 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 |
@@ -84,6 +84,7 @@ export function vitePluginActions({ | |||
code += `\nexport * from 'astro/actions/runtime/virtual/server.js';`; | |||
} else { | |||
code += `\nexport * from 'astro/actions/runtime/virtual/client.js';`; | |||
code = code.replace('__TRAILING_SLASH__', JSON.stringify(settings.config.trailingSlash === 'always')); |
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 sure if this is a correct way to pass config settings
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.
Usually, inside the templates, we have comments like this, which will be then used as placeholders to replace with concrete values by the vite plugins:
astro/packages/astro/templates/env.mjs
Line 32 in 72c1d5d
// @@ON_SET_GET_ENV@@ |
/* @@LOOKUP_MAP_ASSIGNMENT@@ */ |
Do you think we might be able to use the same convention?
Maybe we can refactor the template code in actions.mjs
like this:
const actionComponentPath = /** @TRAILING_SLASH@ **/ ;
const actionPath = import.meta.env.BASE_URL.replace(/\/$/, '')} + '/_actions/' + path + actionComponentPath
CodSpeed Performance ReportMerging #12657 will not alter performanceComparing Summary
|
const pipeline: Pipeline = Reflect.get(context, apiContextRoutesSymbol); | ||
|
||
const baseAction = await getAction( | ||
pipeline.manifest.trailingSlash === 'always' ? callerInfo.name.replace(/\/$/, '') : callerInfo.name, | ||
); |
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.
We have three utilities for manage the trailing slash, you should use them. Here's an example from the source code:
astro/packages/astro/src/core/routing/rewrite.ts
Lines 48 to 50 in 72c1d5d
pathname = shouldAppendForwardSlash(trailingSlash, buildFormat) | |
? appendForwardSlash(newUrl.pathname) | |
: removeTrailingForwardSlash(newUrl.pathname); |
const baseAction = await getAction( | ||
pipeline.manifest.trailingSlash === 'always' ? callerInfo.name.replace(/\/$/, '') : callerInfo.name, | ||
); | ||
// const baseAction = await getAction(callerInfo.name); |
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.
To remove?
@@ -84,6 +84,7 @@ export function vitePluginActions({ | |||
code += `\nexport * from 'astro/actions/runtime/virtual/server.js';`; | |||
} else { | |||
code += `\nexport * from 'astro/actions/runtime/virtual/client.js';`; | |||
code = code.replace('__TRAILING_SLASH__', JSON.stringify(settings.config.trailingSlash === 'always')); |
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.
Usually, inside the templates, we have comments like this, which will be then used as placeholders to replace with concrete values by the vite plugins:
astro/packages/astro/templates/env.mjs
Line 32 in 72c1d5d
// @@ON_SET_GET_ENV@@ |
/* @@LOOKUP_MAP_ASSIGNMENT@@ */ |
Do you think we might be able to use the same convention?
Maybe we can refactor the template code in actions.mjs
like this:
const actionComponentPath = /** @TRAILING_SLASH@ **/ ;
const actionPath = import.meta.env.BASE_URL.replace(/\/$/, '')} + '/_actions/' + path + actionComponentPath
Changes
Closes withastro/adapters#464
Problem: node adapter doesnt pass astro actions request and always redirect this to path with the / in the end
image
Solution: add trailing slash to actions url in case
trailingSlash: "always"
to pass node adapter static handlerTesting
Tested with node adapter and add new test for server actions handler
Docs
Probably no need to update docs