-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 build base bug #3068
fix build base bug #3068
Conversation
🦋 Changeset detectedLatest commit: 5961975 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 |
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.
Looks good to me! Added a question just as a sanity check but this fix + test should have it covered 👍
@@ -191,7 +191,7 @@ async function generatePath( | |||
} | |||
|
|||
const ssr = isBuildingToSSR(opts.astroConfig); | |||
const url = new URL(origin + pathname); | |||
const url = new URL(opts.astroConfig.base + pathname.substring(1), origin); |
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.
I think the answer here is yes, but is it safe to always assume pathname
will start with a /
here?
If not, may be a good spot for a regex that only strips the first character if it's a /
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.
yea, agreed. better safe than sorry
* fix ssr url search params bug * fix build base bug * safer slash removal
Changes
base
configTesting
Docs