Skip to content

Set up redirects based on 404 data & filter sitemap - #1976

Merged
delucis merged 6 commits into
mainfrom
chris/redirects
Nov 3, 2022
Merged

delucis merged 6 commits into
mainfrom
chris/redirects

Conversation

@delucis

@delucis delucis commented Nov 2, 2022 •

Copy link
Copy Markdown
Member

What kind of changes does this PR include?

  • Changes to the docs site code

Description

  • Add a Netlify redirect for an old renderer-reference URL

  • Add a route with a <meta> refresh redirect for /[lang]/tutorial

    We may one day have an actual /tutorial landing page potentially, so using a .astro route with <meta> seemed safer than configuring a Netlify redirect we’d be more likely to forget to remove.

Testing

These URLs were 404s but should now work:

@netlify

netlify Bot commented Nov 2, 2022 •

Copy link
Copy Markdown

✅ Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 8b9574a
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/636404cb19cf140008f6f8f7
😎 Deploy Preview https://deploy-preview-1976--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

const { lang } = Astro.params;
---

<meta http-equiv="refresh" content={`0;url=/${lang}/tutorial/0-introduction/`} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this show up in the sitemap?

@delucis delucis Nov 2, 2022 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Excellent question. The answer is: yes!

https://deploy-preview-1976--astro-docs-2.netlify.app/sitemap-0.xml

We can filter sitemap URLs I believe. Let me look at that — we should probably exclude our other similar redirects too. (Probably doesn’t matter too much — these redirect pages are completely empty so are very unlikely to be indexed, but still.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK, question back time: does it matter if it is in sitemap?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Its usually best to keep redirects out of the sitemap, but that can be tackled in a separate PR 👍

Its really not a huge deal in general, I think it's pretty common for redirects to linger in a sitemap for a while (or forever on some sites)

@delucis delucis Nov 3, 2022 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK — pushed a fix for the low-hanging fruit here. Our [lang]/something.astro redirects like the one added in this PR are easy to filter out. Other dynamic redirects in the root of src/pages/ will be trickier and can be tackled later I guess (or refactored to be configured in netlify.toml).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Engaged morning brain and implemented a much better filtering set-up so now the sitemap shouldn’t include any of our redirect routes!

@sarah11918

sarah11918 commented Nov 2, 2022 •

Copy link
Copy Markdown
Member

No objections here, once you're happy @delucis !

Just putting it on the radar that since we've discussed adding additional content in the "Tutorials" section, maybe it's time for @Jutanium and I to start discussing/sketching out a generic tutorial landing page anyway? I think this could fit in next sprint, actually!

@tony-sull tony-sull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

Comment thread astro.config.ts Outdated
Comment thread integrations/sitemap.ts
/** Get a preconfigured instance of the `@astrojs/sitemap` integration. */
export function sitemap(): AstroIntegration {
return AstroSitemap({
filter: (page) => isValidPath(new URL(page).pathname),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

@delucis delucis changed the title Set up redirects based on initial 404 data Set up redirects based on 404 data & filter sitemap Nov 3, 2022
@delucis
delucis merged commit 322d906 into main Nov 3, 2022
@delucis
delucis deleted the chris/redirects branch November 3, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants