Allow spaces in filenames #1212
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1212 +/- ##
==========================================
- Coverage 88.28% 86.45% -1.84%
==========================================
Files 77 77
Lines 4355 4113 -242
==========================================
- Hits 3845 3556 -289
- Misses 510 557 +47
Continue to review full report at Codecov.
|
src/Resolver.js
Outdated
| @@ -27,7 +27,7 @@ class Resolver { | |||
| } | |||
|
|
|||
| async resolve(input, parent) { | |||
| let filename = input; | |||
| let filename = decodeURIComponent(input); | |||
There was a problem hiding this comment.
I don't think this is the right place to do this. We should only decode when we know that the input is a URL. For example, you could legitimately have a filename called "my%20file" where %20 is not a space.
I think we should do this in addURLDependency where we know the input is supposed to be a URL. Note that we already handled this for HTML assets, so we should move that call into addURLDependency itself so that it applies to e.g. CSS as well. /~https://github.com/parcel-bundler/parcel/blob/master/src/assets/HTMLAsset.js#L90
There was a problem hiding this comment.
Fixed it in the latest commit, now it supports url encoded files from any asset, and doesn't pass url encoded strings to asset.dependencies
src/Asset.js
Outdated
| const resolved = path.resolve(path.dirname(from), parsed.pathname); | ||
| this.addDependency( | ||
| './' + path.relative(path.dirname(this.name), resolved), | ||
| './' + | ||
| path.relative(path.dirname(this.name), decodeURIComponent(resolved)), |
There was a problem hiding this comment.
wouldn't resolved already be decoded here?
There was a problem hiding this comment.
URL.parse url encodes the parsed string.
I've pushed a fix as originally I thought the parsed url's encoding was important (therefore decoding twice), but it isn't as we create new filenames anyway.
Now it only decodes once saving some ms of building.
Closes #873