Add support for source maps #17775
Add support for source maps #17775
Conversation
thecrypticace
force-pushed
the
feat/v4-source-maps-v2
branch
3 times, most recently
from
ebf39e4 to
865f2cc
Compare
Source map support needs this in loadStylesheet but it makes more sense to be consistent and require it for both.
Now that we can handle CRLF correctly in the parser this is no longer necessary
These are stored as character offsets for the original input (`src`) as well as the printed output (`dst`)
thecrypticace
force-pushed
the
feat/v4-source-maps-v2
branch
from
865f2cc to
502bda7
Compare
There was a problem hiding this comment.
This looks really sick. Leaving an early review here. It would be cool if you can include a summary of the implementation in the PR description too including lining out how to test things locally a bit (I assume I can just enable a dev build of Vite with css source maps and then look at the dev tools? I haven't looked into that yet, though)
| return { | ||
| source: pos.source, | ||
| original: source | ||
| ? source.slice(originalOffset, originalOffset + 10).trim() + '...' |
There was a problem hiding this comment.
what does the +10 do here?
There was a problem hiding this comment.
This is just to slice out part of the source so we're not printing the entire thing. It's basically a "does it start with the right stuff" check.
With the convo Robin and I had about visualizing the source maps though I'll probably change these tests and the unit tests to all use the same visualization mechanism — once I've figured out how to actually do it that is. :D
|
|
||
| // Work around an issue where the media query range syntax transpilation | ||
| // generates code that is invalid with `@media` queries level 3. | ||
| let magic = new MagicString(code) |
There was a problem hiding this comment.
Wait do we need all of the MagicString dependency only to track the replaceAll below? 😭
There was a problem hiding this comment.
Yep. 😭 The only reason magic-string AND @ampproject/remapping are deps is for the replaceAll to fix that lightning bug. But also lightning is currently has broken source map locations with license comments anyway (my PR is still open there I think)
| expect(annotations).toEqual([ | ||
| 'index.css: 1:0-41 <- 1:0-41', | ||
| 'index.css: 2:0-13 <- 3:0-34', | ||
| 'theme.css: 3:2-15 <- 1:0-15', |
There was a problem hiding this comment.
Can we somehow test this in a different way? I remember from the v3 codebase that every once in a while when we made a change we had to update this table and it was just updating numbers and very hard to verify if this was actually correct.
Not sure what it would actually look like though 🤔
Maybe something like
.foo { | .foo {
text-decoration-line: underline; | @apply underline;
} | }
There was a problem hiding this comment.
Other/additional idea: if the input source maps and output source maps are the same, don't even try to visualize it. But if they are different (due to printing, whitespace handling or like @apply) then only visualize things.
There was a problem hiding this comment.
Other visualization idea:
/* Source */
.foo {
000000
@apply underline;
11111111111111111
}
/* Generated */
.foo {
000000
text-decoration-line: underline;
11111111111111111111111111111111
}
😂
Use 0-9, a-z, A-Z. OR you could go nuts.
There was a problem hiding this comment.
Another one:
/* Source */
.foo {
------ #0
@apply underline;
----------------- #1
}
/* Generated */
.foo {
------ #0
text-decoration-line: underline;
-------------------------------- #1
}
Or for multiple ones:
/* Source */
.foo {
------ #0
@apply underline flex;
--------- #1
---- #2
}
/* Generated */
.foo {
------ #0
display: flex;
-------------- #2
text-decoration-line: underline;
-------------------------------- #1
}
Could also use a gutter:
/* Source */
.foo {
#0 ------
@apply underline flex;
#1 ---------
#2 ----
}
/* Generated */
.foo {
#0 ------
display: flex;
#2 --------------
text-decoration-line: underline;
#1 --------------------------------
}
Either way, not necessary for this PR
Closes #13694
This is still a big work in progress but opening this so I can get some eyes on it. The core of the implementation is done and works for the CLI as well as Vite. PostCSS is still very much up in the air right now. Adding source locations to it is going to be quite challenging.