Fix Windows Proxy bug #565
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/8pq4m8otf |
drwpow
force-pushed
the
drwpow/resource-test
branch
from
e9e1181
to
9c4fc59
Compare
drwpow
changed the title
drwpow
force-pushed
the
drwpow/resource-test
branch
from
9c4fc59
to
13fbe72
Compare
drwpow
changed the title
drwpow
force-pushed
the
drwpow/resource-test
branch
from
13fbe72
to
d52c93b
Compare
| ), | ||
| ), | ||
| format(readFileSync(path.join(entry.path1, entry.name1), {encoding: 'utf8'})), | ||
| format(readFileSync(path.join(entry.path2, entry.name2), {encoding: 'utf8'})), |
There was a problem hiding this comment.
Opinionated test runner cleanup 🤷 Ran across this while making sure there wasn’t any Windows-specific fixes in the runner itself
| @@ -368,7 +368,7 @@ export async function command(commandOptions: CommandOptions) { | |||
| for (const proxiedFileLoc of allProxiedFiles) { | |||
| const proxiedCode = await fs.readFile(proxiedFileLoc, {encoding: 'utf8'}); | |||
| const proxiedExt = path.extname(proxiedFileLoc); | |||
| const proxiedUrl = proxiedFileLoc.substr(buildDirectoryLoc.length); | |||
| const proxiedUrl = proxiedFileLoc.substr(buildDirectoryLoc.length).replace(/\\/g, '/'); // replace backslashes on Windows | |||
There was a problem hiding this comment.
This is the solution here… but how do we feel about it?
There was a problem hiding this comment.
+1! We do this in a couple of other places, I don't hate it.
There are npm packages for converting any file path to a valid URL. In a future PR, we should look into using one of those for everthing that we do.
There was a problem hiding this comment.
Would it make sense to pull it into a util function if it's used in multiple places and would help self document with the function name?
There was a problem hiding this comment.
+1 for this. Something to mark “this is an import meant for the browser so we definitely don’t need backslashes.” We could also theoretically do things like URL escaping, but hopefully we’ll never need that.
Changes
Following this PR in create-snowpack-app that uncovered a bug where proxied resources had backslashes when built on Windows. This PR fixes that.
Testing