-
-
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
Only resolve inline script specifiers in the static build #2302
Conversation
🦋 Changeset detectedLatest commit: 2d416c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✔️ Deploy Preview for astro-www ready! 🔨 Explore the source changes: 5c98456 🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-www/deploys/61d44db1656da000074ca726 😎 Browse the preview: https://deploy-preview-2302--astro-www.netlify.app |
✔️ Deploy Preview for astro-docs-2 ready! 🔨 Explore the source changes: 5c98456 🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-docs-2/deploys/61d44db1c7aa35000862a8e3 😎 Browse the preview: https://deploy-preview-2302--astro-docs-2.netlify.app |
✔️ Deploy Preview for astro-www ready! 🔨 Explore the source changes: f7471ef 🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-www/deploys/61d44dd80bdc290008e032d1 😎 Browse the preview: https://deploy-preview-2302--astro-www.netlify.app |
✔️ Deploy Preview for astro-docs-2 ready! 🔨 Explore the source changes: f7471ef 🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-docs-2/deploys/61d44dd8bd4b7800082bbb5a 😎 Browse the preview: https://deploy-preview-2302--astro-docs-2.netlify.app |
@@ -151,14 +203,36 @@ async function generatePath(path: string, opts: StaticBuildOptions, gopts: Gener | |||
route: pageData.route, | |||
routeCache, | |||
logging, | |||
pathname: path, | |||
pathname, |
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.
On L195 I was like "huh" and on L206 I’m like "ahh". 👍
const filenameURL = new URL(`file://${id}`); | ||
const pathname = filenameURL.pathname.substr(config.projectRoot.pathname.length - 1); |
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.
Nit Idea: Is filenameURL
being used to sanitize the URL? If so, I think a future PR should introduce posix sanitation methods. Then this line could be like:
/** Pathname, with a leading slash. */
const pathname = posix.path(id).slice(config.projectRoot.pathname.length - 1)
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.
It's being used to turn it into the pathname instead yeah. Yeah that way would presumably work too.
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.
Generally, I follow. 👍
…2302) * Revert "Revert "Implement hydrated components in the static build (withastro#2260)"" This reverts commit 17ac18e. * Only resolve specifiers in the static build * Adding a changeset * Fix the client-only test
Changes