-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: strip query string before checking md extension #12712
Conversation
🦋 Changeset detectedLatest commit: 38120e5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@@ -18,9 +18,11 @@ export function isURL(value: unknown): value is URL { | |||
} | |||
/** Check if a file is a markdown file based on its extension */ | |||
export function isMarkdownFile(fileId: string, option?: { suffix?: string }): boolean { | |||
// Strip query string | |||
const id = fileId.split("?")[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have a stripQueryParams util somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find one. There's getFileInfo
which does return an id with the params stripped - and which the markdown plugin calls immediately after, but it does lots of other stuff which is probably overkill for something that will be called on every module. Better to have something extra simple when it's in the kinda-hot path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do! :) removeQueryString
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh!
CodSpeed Performance ReportMerging #12712 will not alter performanceComparing Summary
|
Changes
Currently the markdown vite plugin just checks if the id ends in a makrdown extension. This breaks if the id includes a query param. This PR strips the query string before checking.
Fixes #12711
Testing
Added test
Docs