Fix slashes for paths containing non-ASCII characters on Windows. #4712
Conversation
🦋 Changeset detectedLatest commit: cc8a7df 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
|
Thanks @Lifeni, can you add a test? If you look in |
|
@matthewp Sorry, I'm new to pull requests and don't have much experience. It seems that I should not merge |
pnpm-lock.yaml
Outdated
| @@ -3225,7 +3231,7 @@ packages: | |||
| dependencies: | |||
| '@astro-community/astro-embed-twitter': 0.1.3_astro@packages+astro | |||
| '@astro-community/astro-embed-youtube': 0.1.1_astro@packages+astro | |||
| astro: link:packages/astro | |||
| astro: link:packages\astro | |||
There was a problem hiding this comment.
not sure why these changed, can you fix them?
There was a problem hiding this comment.
Thanks, I will fix them.
|
@Lifeni no need to merge anything. I'll take care of that. If you can just make the updates requested, thanks! |
| const html = await fixture.readFile(`/index.html`); | ||
| const $ = cheerio.load(html); | ||
|
|
||
| expect($('h1').text()).to.equal('测试 OK'); |
| @@ -126,7 +126,7 @@ export function resolveDependency(dep: string, projectRoot: URL) { | |||
| * Windows: C:/Users/astro/code/my-project/src/pages/index.astro | |||
| */ | |||
| export function viteID(filePath: URL): string { | |||
| return slash(fileURLToPath(filePath) + filePath.search); | |||
| return slash(fileURLToPath(filePath) + filePath.search).replace(/\\/g, '/'); | |||
There was a problem hiding this comment.
Do we still need to use slash if we're always doing replace ourselves now?
Changes
Related issue: #4689
The
slashlibrary cannot handle paths containing non-ASCII characters, so we need to manually replace\with/.Examples
Testing
Test locally only.
Docs
None.