Skip to content

Conversation

@jhnns
Copy link
Member

@jhnns jhnns commented Feb 13, 2017

  • Make all source map paths relative to the entry file. This way we don't have to read the context option from the webpack config.

  • Fix source maps on Windows: node-sass returns POSIX paths, that's why we need to transform them back to native paths. This fixes an error on windows where the source-map module cannot resolve the source maps.

See #366 (comment)

- Make all source map paths relative to the entry file. This way we don't have to read the `context` option from the webpack config.

- Fix source maps on Windows: node-sass returns POSIX paths, that's why we need to transform them back to native paths. This fixes an error on windows where the source-map module cannot resolve the source maps.

See #366 (comment)
@jhnns
Copy link
Member Author

jhnns commented Feb 13, 2017

@bholloway this will again be breaking for the resolve-url-loader. I've created a PR for this: bholloway/resolve-url-loader#44

@jhnns jhnns merged commit e5c25af into master Feb 13, 2017
@jhnns jhnns deleted the refactor/source-map branch February 13, 2017 19:17
@bholloway
Copy link

@jhnns Can you confirm that module imports (i.e. tilde ~) are relative to the entry file? I am seeing otherwise.

I will try to reproduce with your /test project.

@bholloway
Copy link

@jhnns I have tweaked your bootstrapSass test to debug the source-maps.

The source code is in this gist. It requires you to install adjust-sourcemap-loader.

The overall output is of the form:

{
  "sources": [
    "webpack:///webpack:///test/scss/bootstrap-sass.scss",
    "webpack:///webpack:///test/scss/~/bootstrap-sass/assets/stylesheets/_bootstrap.scss"
    ...
  ]
  ...,
  "sourceRoot": ""
}

We can ignore the repeated webpack:/// protocol for now. It has been a while since I debugged source-maps but the tilde in the path certainly feels wrong.

I am less interested in the overall output and more interested in what goes into the resolve-url-loader.

The debug output from adjust-sourcemap-loader shows the source-map following sass-loader (i.e. at its input):

INPUT bootstrap-sass.scss
      node_modules/bootstrap-sass/assets/stylesheets/_bootstrap.scss
      node_modules/bootstrap-sass/assets/stylesheets/bootstrap/_variables.scss
      ...

While this debug info does not include the sourceRoot I believe a "sourceRoot": "test/scss" would produce the overall output we see above. However you can see there is no common source root possible for these paths.

(Please Note the adjust-sourcemap-loader will try to locate files using all the CODECs at its disposal. That is the ABSOLUTE path in the debug output. However that does not help us here.)

jhnns added a commit that referenced this pull request Feb 14, 2017
All source map paths will be relative to process.cwd() from now on.
This removes also the last dependency on this.options.context.
node-sass source map options like sourceMapRoot, omitSourceMapUrl, sourceMapContents are now overridable.

#374 (comment)
@jhnns
Copy link
Member Author

jhnns commented Feb 14, 2017

These source maps are truly driving me crazy ^^

Could you review #377 and try it with your test setup? I think, now all paths should be correct.

@bholloway
Copy link

@jhnns There is good news and bad news. I will comment on #377 because it is still open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants