Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sourcemap paths fail because of overriden sourceMap option. #484

Closed
ghost opened this issue Jul 11, 2017 · 15 comments
Closed

Sourcemap paths fail because of overriden sourceMap option. #484

ghost opened this issue Jul 11, 2017 · 15 comments

Comments

@ghost
Copy link

ghost commented Jul 11, 2017

I have a folder structure like this:

  • <Project root>
    • Assets
      • css
        • bundles
        • modules

Webpack is run from <Project root> with the config file residing in <Project root>/Assets

In normalizeOptions.js the sourceMap option is overriden like this:

options.sourceMap = path.join(process.cwd(), "/sass.map")

All SourceMap paths of imported files look like this:
./Assets/css/bundles/Assets/css/modules/utils/_mixins.scss
This is very wrong.

If i comment out that line and set sourceMap option manually, i can get sourceMaps to work:

loader: "sass-loader",
options: {
  sourceMap: path.resolve(__dirname, "css/bundles/bundle.css")
}

SourceMap paths now look correct:
./Assets/css/modules/utils/_mixins.scss

@alexander-akait
Copy link
Member

@baunegaard your can set sourceMapRoot to <Project root>/Assets. If your provide minimal test repo it is help to solve problem.

@ghost
Copy link
Author

ghost commented Jul 11, 2017

@evilebottnawi just tried that. Makes no difference. Paths are still mangled.

Will try to put up a test repo.

@ghost
Copy link
Author

ghost commented Jul 11, 2017

@evilebottnawi
Could solve this by just giving the user the opportunity to choose a path them self.

// Dont override sourceMap option if a specific path is given.
if (typeof options.sourceMap === "boolean") {
	// Deliberately overriding the sourceMap option here.
	// node-sass won't produce source maps if the data option is used and options.sourceMap is not a string.
	// In case it is a string, options.sourceMap should be a path where the source map is written.
	// But since we're using the data option, the source map will not actually be written, but
	// all paths in sourceMap.sources will be relative to that path.
	// Pretty complicated... :(
	options.sourceMap = path.join(process.cwd(), "/sass.map");
}

With a minimal test repo do you mean just an empty repo with a webpack.config to reproduce? Or a pull request with tests inside the sass-loader repo? Sorry not that used to doing open source development :)

@alexander-akait
Copy link
Member

alexander-akait commented Jul 11, 2017

@baunegaard

With a minimal test repo do you mean just an empty repo with a webpack.config to reproduce

@ghost
Copy link
Author

ghost commented Jul 11, 2017

@evilebottnawi I have reproduced the problem in the following repo:
https://github.com/baunegaard/sass-loader-sourcemap-test

The mangled path looks like this in chrome:
capture

Apply the fix from the OP and problem is gone.

@alexander-akait
Copy link
Member

@baunegaard Did not understand what expected behavior? webpack always story source maps in webpack://. Your want change this? If yes, why?

@ghost
Copy link
Author

ghost commented Jul 11, 2017

@evilebottnawi No look at the path to the sourcemap in the screenshot: "./css/bundles/css/modules/module.scss"

This is not correct. The correct path should be "./css/modules/module.scss"

If you look at the repo, the directory structure is:

  • <Project Root>
    • css
      • bundles
        • bundle.scss
      • modules
        • module.scss
    • node_modules

The webpack entry point is "<Project Root>/css/bundles/bundle.scss" which has two imports:
@import "~bootstrap/dist/css/bootstrap.css";
@import "../modules/module.scss";

node_modules resolve correct.

@alexander-akait
Copy link
Member

@baunegaard thanks for clarify, try to found solution in near future

@michael-ciniawsky
Copy link
Member

@baunegaard Your issue is that webpack.config.js isn't in <Project Root> so process.cwd() will resolve to <Project Root>/Assets/path/to/webpack.config.js which is appended internally by webpack, could you try by temporarily moving webpack.config.js in <Project Root> to verify please? I think your fix is correct, it should resolve to an absolute path and using __dirname instead of process.cwd(), but not 💯 tbh

@alexander-akait
Copy link
Member

alexander-akait commented Jul 12, 2017

We can fix it change result.map.sources = result.map.sources.map(path.normalize) to result.map.sources = result.map.sources.map((source) => path.normalize(path.resolve(source)));, but I'm not completely sure, I'm still looking for a solution.

But it is should be work with relative path, seems problems not in sass-loader, we return correct sources:

[ 'css/bundles/bundle.scss', 'css/modules/module.scss' ]
[ 'node_modules/bootstrap/dist/css/bootstrap.css' ]

@alexander-akait
Copy link
Member

alexander-akait commented Jul 12, 2017

Using (here https://github.com/webpack-contrib/sass-loader/blob/master/lib/loader.js#L73):

console.log(result.map.sourceRoot)
console.log(result.map.sources)

Output:

/home/evilebottnawi/IdeaProjects/sass-loader-sourcemap-test
[ 'css/bundles/bundle.scss', 'css/modules/module.scss' ]
/home/evilebottnawi/IdeaProjects/sass-loader-sourcemap-test
[ 'node_modules/bootstrap/dist/css/bootstrap.css' ]

All fine, seems issue related to css-loader, i will try to found what is wrong and why it is happens in css-loader

@ghost
Copy link
Author

ghost commented Jul 12, 2017

@michael-ciniawsky Tried moving config to project root. Did not make a difference.
I guess that is because i already run webpack from project root like this:

"scripts": {
  "dev": "./node_modules/.bin/webpack-dev-server --config ./Assets/webpack.development.js",
}

As a temporary workaround i used adjust-sourcemap-loader to fix the paths.

Now my only problem is that stars (*) gets appended to some of the source map sources somewhere in the chain. This does not break sourcemaps in general, but it does break Chrome DevTool Workspaces as it wont map to the original files.

@alexander-akait
Copy link
Member

alexander-akait commented Jul 12, 2017

@baunegaard related to css-loader, because from for postcss set as /css-loader!/path/to/css/bundles/bundle.scss (https://github.com/webpack-contrib/css-loader/blob/master/lib/processCss.js#L198) and all sources interpreted (when postcss build source maps) as relative to this path, but in your case module.css not relative

@ghost
Copy link
Author

ghost commented Jul 12, 2017

@evilebottnawi I finally got source maps working!
By changing this line: https://github.com/webpack-contrib/css-loader/blob/master/lib/processCss.js#L198
to:

from: options.from,

thereby allowing path rewriting of sources in css-loader, all problems were fixed!

I now have correct paths to all sourcemaps, even without my first change to sass-loader. So the problem definitely seems to be in css-loader.

Only thing was 2 extra duplicate source maps located in the root.
By using adjust-sourcemap-loader like this:

var templateFn = require("adjust-sourcemap-loader")
	.moduleFilenameTemplate(
	{
		format: "webpackProtocol"
	});

and changing webpack.config.js output to:

devtoolModuleFilenameTemplate: templateFn,
devtoolFallbackModuleFilenameTemplate: templateFn

eveything now looks perfect! No duplicate sourcemaps and all sourcemap paths are correct.
I even added an extra loader to the chain so it is now: css-loader -> postcss-loader -> sass-loader

@alexander-akait
Copy link
Member

Close here because not related to sass-loader, thanks all!

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

No branches or pull requests

2 participants