-
-
Notifications
You must be signed in to change notification settings - Fork 60
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 url concatenation for dev mode with origin set in server options #732
Conversation
🦋 Changeset detectedLatest commit: ead219f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
packages/vite/src/index.ts
Outdated
@@ -1,5 +1,5 @@ | |||
import { basename, extname } from 'node:path' | |||
import { join } from 'node:path/posix' | |||
import urlJoin from 'url-join' |
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 don't think we need to add a dependency for such a simple piece of functionality. Can you just add a method here to do it instead?
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 changed it to string concatenation. This should work fine, because Vite makes sure that the origin option never ends with a '/'.
5415271
to
f6197ca
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #732 +/- ##
==========================================
- Coverage 95.69% 95.69% -0.01%
==========================================
Files 33 33
Lines 1278 1277 -1
Branches 224 224
==========================================
- Hits 1223 1222 -1
Misses 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This change contains a bug fix for the feature introduced in pull request Add vite
origin
support for image urls in development #690. The server origin option usually contains a protocol scheme identifier followed by a double slash, e.g. "http://localhost:1234". Nodes POSIX path implementation is not meant to join URLs, which means the double slash is converted to a single slash, resulting in an invalid URL, e.g "http:/localhost:1234". Using url-join instead ensures the result will be a valid URL.Other information:
Added new dependency: url-join, which seams to be the safest option