Fix injected route edge case #7399
Conversation
🦋 Changeset detectedLatest commit: d4b2a9f 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
bot
added
the
pkg: astro
| // this must be last | ||
| .replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}.mjs`; | ||
| .replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}`; | ||
| const name = pages[pageModuleId]?.route?.route ?? pageModuleId; |
There was a problem hiding this comment.
I think this must be done at the end? Here pageModuleId has its name still mangled and it won't match any route
There was a problem hiding this comment.
The first step is to unmangle the facadeModuleId so we can match it to a route. After that, we mangle the [ and ] and ... characters.
|
Even though all tests should pass, I'm surprised we hit a regression. Isn't there a way to simulate a test for the issue that was filed? |
| import { expect } from 'chai'; | ||
| import { loadFixture } from './test-utils.js'; | ||
|
|
||
| describe('Custom 404 with injectRoute from dependency', () => { |
There was a problem hiding this comment.
Added a specific test for #7397! The injected route will be matched to a relative path in ./node_modules but the build should still succeed. This test verifies that this is the case.
Changes
>=2.6.4#7397node_modulesTesting
Existing tests should pass, additional
injectRoutefrom packages addedDocs
No, bug fix only