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.

Sign up for GitHub

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

Jump to bottom

fix: generated redirect page canonical lacks of site prefix #8591

Merged
merged 14 commits into from
Sep 25, 2023
Merged

fix: generated redirect page canonical lacks of site prefix #8591

merged 14 commits into from
Sep 25, 2023

Conversation

rishi-raj-jain
Copy link
Contributor

@rishi-raj-jain rishi-raj-jain commented Sep 18, 2023 •
edited
Loading

fix #8499

Changes

Fix generated redirect page canonical lacks of site prefix

Testing

Docs

@changeset-bot
Copy link

changeset-bot bot commented Sep 18, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 88839c9

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

@rishi-raj-jain rishi-raj-jain changed the title add site url to the redirect fix: generated redirect page canonical lacks of site prefix Sep 18, 2023
@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Sep 18, 2023
ematipico
ematipico reviewed Sep 20, 2023
View reviewed changes
Copy link
Member

@ematipico ematipico left a comment •
edited
Loading

Choose a reason for hiding this comment

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

can we add a test please? could you restore the issue template please?

@rishi-raj-jain
Copy link
Contributor Author

rishi-raj-jain commented Sep 20, 2023 •
edited
Loading

@ematipico

Can you guide me on writing tests?

I've restored the issue template.

ematipico
ematipico requested changes Sep 22, 2023
View reviewed changes
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

For a test, you could create a new test case inside static-build.test.js, and use the fixture ./fixtures/static-build/.

Then it's just a matter of creating a new page to recreate the issue, and assert that the canonical attribute contains the expected string.

packages/astro/src/core/build/generate.ts Outdated Show resolved Hide resolved
@rishi-raj-jain
Copy link
Contributor Author

rishi-raj-jain commented Sep 23, 2023 •
edited
Loading

For a test, you could create a new test case inside static-build.test.js, and use the fixture ./fixtures/static-build/. Then it's just a matter of creating a new page to recreate the issue, and assert that the canonical attribute contains the expected string.

Thanks! That helped, pushed!

@ematipico ematipico merged commit 863f517 into withastro:main Sep 25, 2023
@astrobot-houston astrobot-houston mentioned this pull request Sep 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ematipico ematipico ematipico approved these changes

Assignees
No one assigned
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

generated redirect page canonical lacks of site prefix
2 participants
@rishi-raj-jain @ematipico