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.

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: add noindex to server island headers #12827

Merged
merged 7 commits into from
Jan 2, 2025
Merged

Conversation

sinskiy
Copy link
Contributor

@sinskiy sinskiy commented Dec 24, 2024

Changes

Sets X-Robots-Tag: noindex to Server Island headers

Fixes #12806

Testing

Added tests that check X-Robots-Tag: noindex of Server Island in both production and development environments

Docs

No changes needed? This just fixes a bug

Copy link

changeset-bot bot commented Dec 24, 2024

🦋 Changeset detected

Latest commit: 7667cc2

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Dec 24, 2024
Copy link

codspeed-hq bot commented Dec 24, 2024

CodSpeed Performance Report

Merging #12827 will not alter performance

Comparing sinskiy:main (7667cc2) with main (7fb2184)

Summary

✅ 4 untouched benchmarks

@matthewp
Copy link
Contributor

Can you try setting it inside of this function instead:

const page: AstroComponentFactory = async (result) => {

This is the handler for server islands, would be cleaner not to put SI specific code in render-context if we can avoid it.

@sinskiy
Copy link
Contributor Author

sinskiy commented Dec 28, 2024

Accidentally updated branch, didn't know what this button does

@sinskiy
Copy link
Contributor Author

sinskiy commented Dec 31, 2024

I think we don't need this PR. see review conversation

@ematipico
Copy link
Member

@sinskiy I updated the PR by fixing the tests (you didn't update them). Now it passes

@ematipico ematipico merged commit 7b5dc6f into withastro:main Jan 2, 2025
15 of 16 checks passed
@sinskiy
Copy link
Contributor Author

sinskiy commented Jan 2, 2025

@ematipico Thanks! But, as I said, x-robots-tag was set to noindex even if I deleted my changes. That's why I didn't push updated tests. I'm confused :/

@matthewp
Copy link
Contributor

matthewp commented Jan 2, 2025

@sinskiy I wonder if you were seeing that because your tests were cached or something, making it seem like the header was already there. You could try submitting a PR that removes your change and see if it now fails. If it doesn't that would mean the header is already there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Server Islands get indexed by crawlers
3 participants