Assorted Source Maps patches #255
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…path. This changes the `source` entry of each map to a path that can be used to get the original source of the asset when the browser requests it through thanks to the source map.
The sources here aren't standalone assets and don't need to properly loaded to be added as links of the generated source map.
The original `source` values are the absolute file paths of each imported/processed stylesheet, which must be replaced by their `source_path` counterpart so we can serve its content to the browser.
rafaelfranca
added a commit
that referenced
this pull request
Feb 26, 2016
Assorted Source Maps patches
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pull Request contains 3 slightly related patches to the Source Maps support related to Babel/Sass assets.
sourcepath on source maps from theBabelProcessorso the browser can request the asset original source, replacing the relative path that was used before.sourcewhen processing the source map. This was causing issues on my app where Sprockets was getting lost on trying to load.cssfiles as.scssones - considering that theSourceMapProcessoralready has thelinkvalue for each source, we can use it instead of (re)loading the file.@imported files (thesourcevalue ends up being the absolute path for each stylesheet that was imported). I'm not 100% happy with callingresolve!+loadinside the processor, but was the only way to transform the absolute file path into the source path I could think of. This patch can be easily test on a public app like Code Triage by inspecting the loaded sources. I haven't tested if the same happens with the Sassc processor, but we can duplicate the test case and see how it goes.Let me know what makes sense/doesn't make sense of these patches, I can remove/rework any of those commits as we need.
I still believe that the source maps support here isn't 100%, as some of my exploratory tests on Google Chrome has showed some mismatches between the source contents and the positions reported by the console, but I don't have enough experience on Source Maps to rule this as an issue with the generated map or with the browser support - but I think those patches are a step forward on this.
Thanks @rafaelfranca for walking through and explaining the current implementation when debugging the first two patches with me yesterday ❤️.