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

use absolute paths and remove sourceRoot when generating sourcemaps #430

Merged
merged 1 commit into from
Mar 6, 2017

Conversation

brokenmass
Copy link
Contributor

@brokenmass brokenmass commented Feb 24, 2017

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
Yes

If relevant, did you update the README?
Not relevant

Summary
Simplifies and correct sourcemaps generation by returning absolute sources paths and removing sourceRoot.
Fix #373 and #280

Does this PR introduce a breaking change?
No

Other information

@codecov
Copy link

codecov bot commented Feb 24, 2017

Codecov Report

Merging #430 into master will decrease coverage by -0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #430      +/-   ##
==========================================
- Coverage   98.35%   98.32%   -0.03%     
==========================================
  Files           9        9              
  Lines         304      299       -5     
  Branches       69       67       -2     
==========================================
- Hits          299      294       -5     
  Misses          5        5
Impacted Files Coverage Δ
lib/loader.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7317ca...13cf191. Read the comment docs.

@brokenmass
Copy link
Contributor Author

any updates ?

@joshwiens
Copy link
Member

@brokenmass for reference ping @webpack-contrib/org-maintainers as it's easy to miss comments in the news feed and watching 43 repositories is a nightmare :)

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-ciniawsky michael-ciniawsky merged commit aacce32 into webpack-contrib:master Mar 6, 2017
@jasisk
Copy link

jasisk commented Mar 17, 2017

This change appears to introduce some complexity for using, e.g., chrome dev tools.

Because css-loader is no longer following the same convention as webpack's source mapping, you end up having to add multiple workspace mappings. I presently store my css alongside the component it corresponds to. I could not figure out the exact incantation of mappings to get everything to work.

Additionally, without a protocol, Chrome treats the path as an absolute path after the host. So, on a Mac, it resolves to http://something.website:port/Users/myUserName/....

@joshwiens
Copy link
Member

@jasisk - Can you get this into a new issue so we are sure this can be addressed one way or another

@michael-ciniawsky
Copy link
Member

Yep please open a new issues this is a very inconvenient regression to said it politely, we need to investigate in a clean way 😛

@davicho
Copy link

davicho commented Aug 30, 2017

I find paths absolute slightly inconvenient. As I inspect my css in the dev tools.... the path can be so long it truncates the actual file name - making have to leave my work are to hover of the path and get the tooltip to see the actual file name I need to work on. (Unless, there is an option to undo this but I have looked and have not seen such). Ty.
truncated-path

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.

Problem with hard-coded sourceRoot
6 participants