Make relative applied source paths relative to CWD #41
Conversation
When generating a source map, the paths put into the `sources` array of the source map normally come from `pos.source` properties on the AST nodes. These are today assumed to be relative to the current working directory, since that’s how any original source maps are resolved and applied. Paths in the `sources` array can also come from applied original maps. Relative such source paths are relative to the original source map -- not to the current working directory! It does not make sense to mix paths that are relative to different things. Therefore this commit makes sure that applied relative source maps are rewritten to be relative to the current working directory, like all other sources. An existing test was updated to test this.
| */ | ||
|
|
||
| function dirname(url) { | ||
| return url.replace(/\/?[^/]+$/, '') || '.'; |
There was a problem hiding this comment.
Is there any reason not to use regular path operations and then normalize to URLs when setting the source name? I think all that is needed is just replacing \ with / to convert to URL, and that is only needed on Windows.
There was a problem hiding this comment.
To clarify, I mean treating the source property of the AST as a file path and then converting that (and any paths derived from that) to URLs only when adding the path to the source map. This would allow using the regular Node path module functions.
There was a problem hiding this comment.
.applySourceMap() requires a URL as its third argument; a Windows-style path does not work. originalMap.sourcesRelativeTo is guaranteed to be a URL, regardless of operating system. Using path.dirname() on it would change it to a Windows-style path when on Windows. Which would require us to change it back to a URL (source maps should contain URLs since they are used in the browser). So I simply chose between two different regexes :)
But you are right: We should probably use standard path operations, and normalize all paths added to the source map to URLs—also in exports.emit. I’d recommend urix.
|
Please can you apply this against reworkcss/css (the new home for parse/stringify development) |
When generating a source map, the paths put into the
sourcesarray ofthe source map normally come from
pos.sourceproperties on the ASTnodes. These are today assumed to be relative to the current working
directory, since that’s how any original source maps are resolved and
applied.
Paths in the
sourcesarray can also come from applied original maps.Relative such source paths are relative to the original source map --
not to the current working directory!
It does not make sense to mix paths that are relative to different
things. Therefore this commit makes sure that applied relative source
maps are rewritten to be relative to the current working directory, like
all other sources.
An existing test was updated to test this.