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

Sourcemaps pointing to wrong file #1078

Closed
JParkinson1991 opened this issue Jun 26, 2019 · 15 comments
Closed

Sourcemaps pointing to wrong file #1078

JParkinson1991 opened this issue Jun 26, 2019 · 15 comments

Comments

@JParkinson1991
Copy link

  • Operating System: macOS 10.14.5
  • Node Version: 8.16.0
  • NPM Version: 6.4.1
  • webpack Version: 4.35.0
  • sass-loader Version: 7.1.0

It seems that there (somewhere in the chain) are some alterations to node-sass generated sourcemaps which results in them pointing to the wrong scss source declarations.

The sourcemaps do not point to completely the wrong file, the point to a file/line in which styling exists for the element, just not the file/line where the shown styles have been defined.

Assumptions

  • When style definitions exist in multiple files, sourcemaps point to one of (but in some cases) the wrong file in which they are defined.
  • This only seems to be the case when multiple style definitions exist for an element

Screenshots

Webpack generated sourcemap - broken - chrome browser
webpack sourcemap - broken

node-sass generated sourcemap - working - chrome browser
Screenshot 2019-06-26 at 15 29 37

How Do We Reproduce?

Reproducable repository here:
https://github.com/JParkinson1991/webpack-sourcemaps-debug

The ./public/index.html page in the above repository will show the expected locations of sourcemap references.

These expected locations are correct when the sourcemaps are generated/styles compiled with node-sass directly. This can be achieved by running the following in the above repository npm run script build:node-sass

However, when compiled using webpack and usings its generated sourcemaps the locations do not match up, they are not compeltely wrong in that they reference somewhere in source where this file has been generated, but not the correct point in the code.
Can be checked by running the following in the above repository npm run script build:webpack

Testing

In chrome, when source compiled with node-sass the sourcemaps are correct, they are incorrect when compiled with webpack.

In firefox, sourcemaps are incorrect when source compiled with both webpack and node-sass


If you need anymore information on this one, let me know.
Thanks in advance for any help/input provided on this one!

@JParkinson1991
Copy link
Author

Just to note, as seen in someother issues setting sass-load outputStyle to compressed seems to make things worse.

{
    loader: 'sass-loader',
    options: {
        sourceMap: true,
        outputStyle: 'compressed'
    }
}

Screenshot 2019-06-26 at 15 53 23

See how the sourcemaps now point to the files containing the mixin in the actual style definition.

@file _navbar.scss

.navbar {
  position: relative;
  min-height: $navbar-height; // Ensure a navbar always shows (e.g., without a .navbar-brand in collapsed mode)
  margin-bottom: $navbar-margin-bottom;
  border: 1px solid transparent;

  // Prevent floats from breaking the navbar
  @include clearfix; // !!! -> THIS IS WHAT LIVES IN _clearfix.scss

  @media (min-width: $grid-float-breakpoint) {
    border-radius: $navbar-border-radius;
  }
}

@JParkinson1991
Copy link
Author

JParkinson1991 commented Jun 26, 2019

In all of the above examples .navbar-default points to the correct part of the code in the soucemap.

There is only one style block/definition for .navbar-default in all of the @imported bootstrap-sass source files.

Im starting to believe this issue stems from element styles being defined in multiples.

This doesn't answer the question on why compressed shows the mixin definition file however.

@alexander-akait
Copy link
Member

In firefox, sourcemaps are incorrect when source compiled with both webpack and node-sass

Problem in firefox (known issue)

To be honestly we don't modify source maps from node-sass, so problem in postcss-loader or in your minificator

@JParkinson1991
Copy link
Author

If you look at the repository i posted there is no post css or minification that takes place when producing these errors.

Sass-loader is passed to css-loader then onto mini css extract plugin.

@alexander-akait
Copy link
Member

Looks problem in css-loader/ MCEP, i will looks on this in near future, reproducible test repo is enouth, thanks

@JParkinson1991
Copy link
Author

Great! I don’t know where to start with debugging this but if theres anything i can do to help just let me know!

@alexander-akait
Copy link
Member

switching on sass (dart-sass) solves problem too, continue investigation

@alexander-akait
Copy link
Member

Yep, bug on node-sass side, you can put code:

require('fs').writeFileSync('./style.css', result.css);
require('fs').writeFileSync('./style.css.map', JSON.stringify(result.map));

in render callback into src/index.js and using https://sokra.github.io/source-map-visualization/ to look at the generated source map and it has invalid mapping. Tried to use sassOptions.sourceMap: true and sassOptions.outFile: 'style.css' and it broken also.

@alexander-akait
Copy link
Member

I was wrong, bug in css-loader (in postcss), I will move it to css-loader, postcss output invalid source maps

@alexander-akait alexander-akait transferred this issue from webpack-contrib/sass-loader Apr 17, 2020
@JParkinson1991
Copy link
Author

@evilebottnawi given the bug has now been narrowed down to css-loader, i assume we can rule out dart sass being a fix (ie node-sass being the issue)?

@alexander-akait
Copy link
Member

@JParkinson1991 not sure, using sass (dart-sass) solve issue, maybe something wrong in mapping for node-sass, it is a real hard bug, I don’t even know where we will need to start

@JParkinson1991
Copy link
Author

Yeah it does seem like a tricky bug to track down! Atleast there is some sort of work around in place with using dart sass.

Just for clarity, using dart sass with npm install --save-dev sass is all thats needed? Removing node-sass of course.

I will dig around this soon and verify dart sass can be used as a work around.

@alexander-akait
Copy link
Member

Just for clarity, using dart sass with npm install --save-dev sass is all thats needed? Removing node-sass of course.

Yes

I will dig around this soon and verify dart sass can be used as a work around.

I already checked this and everything works fine, I'm not sure that I can find the time to track down this error in the near future

@JParkinson1991
Copy link
Author

Just updated the reproducible repository to include dart sass test cases. Everything working as expected.

https://github.com/JParkinson1991/webpack-sourcemaps-debug

@alexander-akait
Copy link
Member

Let's close in favor #622, I will add tests to ensure what we fix it after source map improvements

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

No branches or pull requests

2 participants