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

better working with source maps #622

Open
MagicDuck opened this issue Oct 18, 2017 · 19 comments
Open

better working with source maps #622

MagicDuck opened this issue Oct 18, 2017 · 19 comments

Comments

@MagicDuck
Copy link

Do you want to request a feature or report a bug?
bug/feature

What is the current behavior?

The css-loader doesn't put it's stuff under webpack:// as far as I can see: https://github.com/webpack-contrib/css-loader/blob/master/lib/loader.js#L117
image
(Ignore some of the weird absolute paths under webpack://, It's cause my package.json is pointing to a local package)

Also, if you are loading a couple of libraries built with webpack which happen to have the same path for a css file (ex: .../src/mystyles.css), you will get sourcemap collision.

If the current behavior is a bug, please provide the steps to reproduce.
any build using the css-loader would do.

What is the expected behavior?
css-loader should be able to be configured to specify the file template.
This is @sokra 's opinion on the issue here: webpack/webpack#5767 (comment)

If this is a feature request, what is motivation or use case for changing the behavior?
We are building libraries with webpack which we load at runtime in our main app. We want to be able to easily debug issues in development.

Please mention other relevant information such as your webpack version, Node.js version and Operating System.

Associated issue on the webpack side: webpack/webpack#5767

@MagicDuck
Copy link
Author

Also wanted to mention that I find it kind of weird that when the dev searches in the devtool, one of the matches for "foo.css" is actually the code produced by the style-loader (and the other by the css-loader). The regular dev shouldn't care about those. Is there a way to turn those off?

@alexander-akait
Copy link
Member

@MagicDuck feel free to PR

@alexander-akait
Copy link
Member

@MagicDuck friendly ping, any ideas how we can solve this?

@MagicDuck
Copy link
Author

Wasn't there a plan to have special chunk templates to generate "css chunks"? Not sure what happened with that, but I am assuming that would make css loader obsolete...

In any case, if we still need to fix css-loader, my best guess for what needs to happen is to add a new option to the loader for sourcemap template path that gets emitted. It should have a smart defaul, defaulting to something using the library name, like I did for the js side in the associated issue mentioned above. We should also turn off or filter sourcemapping for style loader if possible to avoid confusion.

Unfortunately atm I am too swamped to do a PR....

@alexander-akait
Copy link
Member

@MagicDuck Maybe you can create minimum reproducible test repo and README where describe what you have and what you expected?

@MagicDuck
Copy link
Author

I thought the bug description above is pretty simple and explicit. Anyways, i would but like I said I don't have the time at the moment 😧

@alexander-akait
Copy link
Member

@MagicDuck What is want here, css-loader should in webpack namespace?

@MagicDuck
Copy link
Author

MagicDuck commented Dec 8, 2018

So imagine you are a dev and are working on an app where some of the consumed libraries have their own css and are in turn built with webpack. When you open the dev tool, you:

  1. Want searching for a css file from a particular lib to be straightforward and make sense. So nested under webpack namespace but with the twist that the lib name is part of the path as in sourcemap collision when building as a library webpack/webpack#5767
  2. The inclusion of the lib name as part of the paths prevents collisions in sourcemap when multiple libraries define a css file with the same name.

Of course if you are not building a lib, the lib name should not be part of the path, but path should still be in webpack namespace,

@alexander-akait
Copy link
Member

@MagicDuck hm, hard to understand without examples 😞

@MagicDuck
Copy link
Author

@evilebottnawi If you reference the screenshot in the issue description above, I want the actual css source for foo.css to go under webpack://(sap.orca-sample-plugin)/src/foo.css instead of C:/code/sap-webpack-extensions... when output.devtoolNamespace="sap.sample-orca-plugin". Atm we actually see the cruft from style-loader under that path, which is not very useful.

@alexander-akait
Copy link
Member

@MagicDuck thanks looks like a bug, we need fix it

@alexander-akait alexander-akait modified the milestones: 2.0.1, 2.1.1 Mar 5, 2019
@alexander-akait
Copy link
Member

Related problem: loader doesn't support devtoolModuleFilenameTemplate and other devtool option in output option, we should fix it

@alexander-akait alexander-akait changed the title css sourcemap sources should be namespaced better working with source maps Mar 11, 2019
@alexander-akait alexander-akait modified the milestones: 2.1.1, 2.2.0 Mar 11, 2019
@undsoft
Copy link

undsoft commented Mar 24, 2019

Any updates on this?

It would be really cool to edit styles here:
DevTools - localhost63342qttest2index html_ijt=ue1i7auj3bpuqbdfkak6cnmg46 2019-03-24 14-51-28
and get them updated in source css / scss files.

I've tried playing with this for some time today, but even when I modify the plugin's output to have full file:// url

// Module
exports.push([module.i, "body h1 {\n  background: red;\n  color: black;\n  font-size: 215px;\n}\n", "", {
  "version": 3,
  "sources": ["file:///Users/undsoft/htdocs/qt/test2/test.css"],
  "names": [],
  "mappings": "AAAA;EACE,eAAe;EACf,YAAY;EACZ,gBAAgB;AAClB",
  "file": "test.css",
  "sourcesContent": ["body h1 {\n  background: red;\n  color: black;\n  font-size: 215px;\n}\n"]
}]);

Chrome still doesn't want to save it correctly.

Relevant issue: webpack/webpack#6400

@alexander-akait
Copy link
Member

@undsoft #717 (comment)

@undsoft
Copy link

undsoft commented Mar 25, 2019

@evilebottnawi
This won't work in the way I'm describing.
Editing from Sources tab will work and will correctly save to the original source file, but you need to manually go to Sources tab every time you want to make a change. To me this is marginally better than switching to an IDE.
What will not work is editing styles directly from the Elements tab as in my screenshot.

@alexander-akait
Copy link
Member

Add tests for devtool*

@alexander-akait alexander-akait removed this from the 2.1.1 milestone Jun 4, 2019
@alexander-akait alexander-akait added this to the 4.0.0 milestone Apr 6, 2020
@alexander-akait
Copy link
Member

alexander-akait commented Apr 6, 2020

An example of the broken repository - https://github.com/alexhisen/mobx-forms-demo

Roadmap:

  • Respect the devtools value in a config (webpack keep true/false in the loaderContext.sourceMap option)
  • All sources in source maps should be relative to context (from configuration) and uses webpack:// prefix (maybe be unique prefix to avoid rewritten existing source maps), like do it webpack
  • The sourceRoot should be empty
  • test all issues
  • Add tests from mention issues

Need to solve:

  • [contenthash] is unstable with mini-css-extract-plugin on webpack@4
  • Rewriting urls() break source maps mapping

@alexander-akait
Copy link
Member

Most of issues are solved #622 (comment), I want to do release and get feedback

@arcanis
Copy link

arcanis commented Feb 3, 2023

While it's certainly a little ugly, perhaps the url(...) issue (which still exists) could be addressed by making sure the url(...) length before / after is the same, by reserving some extra space in the generated CSS, and replacing it by empty spaces as needed after resolving the require?

So for example, the following:

background: url(___CSS_LOADER_URL___0___RESERVING_SPACE_FOR_SOURCE_MAPS___);

Would turn into (note how the semicolon position is the same before / after):

background: url("path/to/image.png")                                       ;

it wouldn't be perfect, and it wouldn't work if the url(...) expression is larger than the reserved space, but since those whitespace would be removed anyway by cssnano, it seems it could be a decent workaround.

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

4 participants