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: match index files only by using entire basename #12815

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ericswpark
Copy link

@ericswpark ericswpark commented Dec 23, 2024

The previous code would cause file names like index.xml.ts to be treated as index files, when they aren't (the file basename excluding the extension .ts is index.xml, and index should be the index file). To match index files only, match the entire first half of the basename when splitting by the last occurrence of the period/file extension delimiter.

Pedantically this might not be correct -- i.e. some file extensions actually span multiple delimiters, like index.tar.gz. But in that case it's not an index file as well, so I think this is fine.

Fixes #12675

Changes

  • What does this change?
  • Be short and concise. Bullet points can help!
  • Before/after screenshots can help as well.
  • Don't forget a changeset! pnpm exec changeset

Testing

This was tested with my website repository that I'm using to migrate to Astro. This may need more tests to make sure it is not a breaking change.

Docs

Possibly -- files that were considered index files may not be and vice versa. Although I think this change should only include files that were previously excluded by the .startswith("index.") check.

/cc @withastro/maintainers-docs for feedback!

The previous code would cause file names like `index.xml.ts` to be
treated as index files, when they aren't (the file basename excluding
the extension `.ts` is `index.xml`, and `index` should be the index
file). To match index files only, match the entire first half of the
basename when splitting by the last occurrence of the period/file
extension delimiter.

Pedantically this might not be correct -- i.e. some file extensions
actually span multiple delimiters, like index.tar.gz. But in that case
it's not an index file as well, so I think this is fine.

Fixes withastro#12675
Copy link

changeset-bot bot commented Dec 23, 2024

🦋 Changeset detected

Latest commit: 65516f1

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 23, 2024
Copy link

codspeed-hq bot commented Dec 23, 2024

CodSpeed Performance Report

Merging #12815 will not alter performance

Comparing ericswpark:subpath-non-index-fix (65516f1) with main (6bbb3f5)

Summary

✅ 4 untouched benchmarks

@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Jan 3, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@@ -0,0 +1,5 @@
---
'astro': minor
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a patch because it's a bugfix not new feature

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) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't have any routes that start with index. in subpaths
2 participants