feat: css sourcemap support during dev #7173

sapphi-red · 2022-03-04T10:59:32Z

Description

Adds css sourcemap support with dev server.
This would be useful for debugging.

This PR supports:

CSS imported with <link>
CSS imported with import
Sass / less / Stylus
additionalData option
Vue SFC style tag

related #2830

close #4950

Additional context

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

sapphi-red · 2022-03-04T11:02:48Z

packages/vite/src/node/plugins/css.ts

+    const newContent = await additionalData(source, filename)
+    const ms = new MagicString(source)
+    ms.overwrite(0, source.length, newContent)
+
+    return {
+      content: ms.toString(),
+      map: generateWithAbsoluteFilenameMap(ms, filename)
+    }


Maybe additionalData should be able to return sourcemap?

I think that makes sense. For this code, does using MagicString to replace its entire content produce a useful sourcemap? I've not tested this myself, but am curious about this behaviour.

For this code, does using MagicString to replace its entire content produce a useful sourcemap?

No. It did not produce useful sourcemaps. I was going to remove this after having some advice.

sapphi-red · 2022-03-04T12:28:58Z

packages/vite/src/node/utils.ts

+  // hack for parse broken with normalized absolute paths on windows (C:/path/to/something).
+  // escape them to linux like paths
+  sourcemapList.forEach((sourcemap) => {
+    sourcemap.sources = sourcemap.sources.map((source) =>
+      source ? escapeToLinuxLikePath(source) : null
+    )
+    if (sourcemap.sourceRoot) {
+      sourcemap.sourceRoot = escapeToLinuxLikePath(sourcemap.sourceRoot)
+    }
+  })
+  const escapedFilename = escapeToLinuxLikePath(filename)


Currently, paths used with sourcemap (sources/file/sourceRoot properties) are absolute paths which are obtained from id (/home/user/path/to/something.js or on windows c:/users/user/path/to/something.js).

There is a problem on windows with this.

const resolve = require('@jridgewell/resolve-uri') const resolved = resolve('C:/project/foo.sass', 'C:/project/bar.sass') console.log(resolved) // C:/project/C:/project/foo.sass

To solve this I escaped them like /c/project/foo.sass.

Just trying to understand our options here. Could these paths be made relative to avoid the hack?

If there is a file importing a file from a different drive, it would not be able to express in relative paths.
This can happen in the following situation:

C:/project/src/foo.css imports C:/project/node_modules/pkg/bar.scss.

preserveSymlinks is false

C:/project/node_modules/pkg is a symbolic link to D:/somewhere/pkg

It will not happen normally for pnpm since it does not create cross-drive links.
But if you use npm i C:/absolute/path/to/pkg or yarn add link:C:/absolute/path/to/pkg or pnpm link C:/absolute/path/to/pkg, a symbolic link (on windows accurately a junction) will be created and this can be cross-drive.
Currently I do not have multiple drives in my computer so I did not try it, but it will happen in theory.

Also JS sourcemaps uses absolute path.
If CSS sourcemaps will use relative path, it would be better to use relative paths for JS sourcemaps for consistency.

JS sourcemap does not need this hack because in JS there is only one input file for one output.

sapphi-red · 2022-03-04T12:29:36Z

pnpm-lock.yaml

+  /@jridgewell/resolve-uri/3.0.5:
+    resolution: {integrity: sha512-VPeQ7+wH0itvQxnG+lIzWgkysKIr3L9sslimFW55rHMdGu/qCQ5z5h9zq4gI8uBtqkpHhsF4Z/OwExufUCThew==}
+    engines: {node: '>=6.0.0'}


Updated to include this.
jridgewell/resolve-uri@aa653a6

dominikg · 2022-03-04T20:08:11Z

whoa! thank you so much for working on this. ❤️

for the record: i did a naive take on this before and it had to be rolled back #4951 because it caused #4939

your code reads like it may take care of that already but still worth validating it doesn't hit that again

sapphi-red · 2022-03-06T09:07:27Z

still worth validating

Sorry I did not get the meaning of "validating".
Do you mean that I should check my code? or add a runtime validation?

dominikg · 2022-03-06T09:50:07Z

still worth validating

Sorry I did not get the meaning of "validating". Do you mean that I should check my code? or add a runtime validation?

see reproduction and discussion in #4939
Manual checking that this PR doesn't produce the error described there would be good, testcases that ensure it even better.
Runtime validation isn't needed i think, if we wanted to add that it could be done via debug logging to avoid permanent overhead

patak-dev · 2022-03-12T19:41:18Z

This in incredible @sapphi-red, thanks for pushing this feat forward

patak-dev · 2022-03-13T12:59:02Z

I'm thinking that we could enable css sourcemap support as experimental during build. See comment from @benmccann here: #2830 (comment)

We could have build.cssSourcemap marked as @experimental, following other options like build.cssCodeSplit. This should let us merge this PR as part of the 2.9 beta and gather some real usage. We can then do a final call on the API for 3.0 (maybe all CSS-related options should be under a single css object?). Thoughts?

@dominikg if you have some time to test or add a review for this PR, that would be helpful too.

packages/plugin-vue/src/style.ts

packages/vite/src/node/__tests__/plugins/css.spec.ts

sapphi-red · 2022-03-13T15:59:57Z

This PR only implements it for dev. It needs more work to be able to output sourcemaps for build.

patak-dev · 2022-03-13T16:40:03Z

Tested it locally and it is working well for me. Disregard my previous comment as it doesn't need a new option, I think after some reviews we can merge it and get some testing during the beta.

dominikg · 2022-03-13T23:04:37Z

@dominikg if you have some time to test or add a review for this PR, that would be helpful too.

👍 for starting with dev-mode sourcemaps in the first iteration. build is a lot more involved, see #2830 (comment)

i tried this branch with sveltekit demo app and the css sourcemap was a bit off when looking at it with https://github.com/evanw/source-map-visualization or trying to follow links from chome debugger to source files. This may very well be caused by svelte's css processing or vite-plugin-svelte not providing proper maps. (The virtual css module is a bit tricky)

It did work for a vanilla vite project, though the map was kinda pointless there as the css was not minified so it was a 1-1 mapping.

I also like the idea of taking this on in a beta, not really worried about sourcemaps being wrong sometimes, as long as this PR doesn't break anything else it is a vast improvement over no sourcemaps at all :)

patak-dev · 2022-03-21T23:09:16Z

Merging this PR so we can test it during the beta. We'll release a new beta patch including it tomorrow.

mbjorke · 2022-03-23T07:04:11Z

I tested this in the beta.6 version now and can confirm it works. Nice work.

tyouzu1 · 2022-03-23T07:19:43Z

This PR only implements it for dev. It needs more work to be able to output sourcemaps for build.

I tested beta.6，found an error，
Read the less file if it is an empty file，result.map will be undefined,JSON.parse(result.map) will throw an error #7411
and submit a pr https://github.com/vitejs/vite/pull/7412/files

tyouzu1 · 2022-03-23T07:22:09Z

This PR only implements it for dev. It needs more work to be able to output sourcemaps for build.

I tested beta.6，found an error， Read the less file if it is an empty file，result.map will be undefined,JSON.parse(result.map) will throw an error #7411 and submit a pr https://github.com/vitejs/vite/pull/7412/files

#7412

sapphi-red added 6 commits February 28, 2022 23:53

feat(dev): css sourcemap

732c3bc

feat(dev): sass sourcemap

38c0052

feat(dev): less sourcemap

32090d9

feat(dev): stylus sourcemap

55077e3

dx: add css sourcemap debug output

64742be

fix(dev): direct css request source map

34f138d

Niputi added the feat: css label Mar 4, 2022

sapphi-red added 4 commits March 4, 2022 20:39

feat: vue sfc css sourcemap

31730b2

fix: css additional data sourcemap

91d8bf5

refactor: move genSourceMapUrl function

13c0bb6

fix: better approach for windows hack

3e54315

sapphi-red force-pushed the feat/css-sourcemap-dev branch from 56aaaee to 3e54315 Compare March 4, 2022 12:27

sapphi-red commented Mar 4, 2022

View reviewed changes

sapphi-red added 6 commits March 6, 2022 17:27

test: move sourcemap test

215c9e9

test: css sourcemap

287cceb

test: sass sourcemap

e041930

test: less sourcemap

5201c1f

test: stylus sourcemap

6f7569b

test: css additional data sourcemap

f3d4461

sapphi-red added 5 commits March 6, 2022 18:12

test: vue sfc style sourcemap

da08d19

chore: merge main branch

15c7f35

fix: rewrite .at(-1)

3311d00

fix: linux sourcemap path escape

add2cd6

test: ignore no test for serve only test file

5607a24

test: change toMatchSnapshot to toMatchInlineSnapshot

4ca19b0

Niputi added the p2-nice-to-have Not breaking anything but nice to have (priority) label Mar 6, 2022

fix: sourcemap warning with plain css

0f56319

refactor: use type assertions if it is legal

59dff9e

bluwy reviewed Mar 13, 2022

View reviewed changes

packages/plugin-vue/src/style.ts Outdated Show resolved Hide resolved

packages/vite/src/node/__tests__/plugins/css.spec.ts Outdated Show resolved Hide resolved

patak-dev changed the title ~~feat: css sourcemap with dev~~ feat: css sourcemap support during dev Mar 13, 2022

sapphi-red added 3 commits March 14, 2022 02:35

refactor: rewrite RawSourceMap type

5b0d47b

refactor: remove unused imports

8e479d3

feat: return sourcemap with additionalData

a1e03f7

benmccann mentioned this pull request Mar 17, 2022

CSS sourcemaps are missing sveltejs/kit#841

Open

patak-dev approved these changes Mar 21, 2022

View reviewed changes

patak-dev merged commit 38a655f into vitejs:main Mar 21, 2022

patak-dev added a commit that referenced this pull request Mar 21, 2022

fix: lockfile after #7173

826ff80

patak-dev mentioned this pull request Mar 21, 2022

fix: lockfile after #7173 #7401

Merged

4 tasks

patak-dev added a commit that referenced this pull request Mar 21, 2022

fix: lockfile after #7173 (#7401)

21919d2

This comment was marked as outdated.

Sign in to view

This was referenced Mar 24, 2022

Update vite in examples to 2.9.0-beta.8 storybookjs/builder-vite#288

Merged

Sourcemap for "/iframe.html" points to missing source files #7432

Closed

This was referenced Mar 26, 2022

feat(css): css.devSourcemap option #7471

Merged

chore(plugin-vue): bump vite peer dep to 2.9.0 #7472

Merged

bluwy mentioned this pull request Apr 4, 2022

Whether the CSS source mapping can be configured in the document supplement #6679

Closed

4 tasks

MadLittleMods mentioned this pull request Apr 20, 2022

from id is always cleaned before being passed to the PostCSS plugins #7822

Closed

7 tasks

sapphi-red mentioned this pull request Apr 20, 2022

fix(css): do not clean id when passing to postcss (fix #7822) #7827

Merged

9 tasks

patak-dev mentioned this pull request Feb 24, 2023

fix: use relative paths in sources for transformed source maps #12079

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: css sourcemap support during dev #7173

feat: css sourcemap support during dev #7173

sapphi-red commented Mar 4, 2022 •

edited

sapphi-red Mar 4, 2022

bluwy Mar 13, 2022

sapphi-red Mar 13, 2022

sapphi-red Mar 4, 2022

patak-dev Mar 12, 2022

sapphi-red Mar 13, 2022

sapphi-red Mar 13, 2022 •

edited

sapphi-red Mar 4, 2022

dominikg commented Mar 4, 2022

sapphi-red commented Mar 6, 2022

dominikg commented Mar 6, 2022 •

edited

patak-dev commented Mar 12, 2022

patak-dev commented Mar 13, 2022

sapphi-red commented Mar 13, 2022 •

edited

patak-dev commented Mar 13, 2022

dominikg commented Mar 13, 2022 •

edited

patak-dev commented Mar 21, 2022

mbjorke commented Mar 23, 2022

This comment was marked as outdated.

tyouzu1 commented Mar 23, 2022

tyouzu1 commented Mar 23, 2022

feat: css sourcemap support during dev #7173

feat: css sourcemap support during dev #7173

Conversation

sapphi-red commented Mar 4, 2022 • edited

Description

Additional context

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

sapphi-red Mar 4, 2022

Choose a reason for hiding this comment

bluwy Mar 13, 2022

Choose a reason for hiding this comment

sapphi-red Mar 13, 2022

Choose a reason for hiding this comment

sapphi-red Mar 4, 2022

Choose a reason for hiding this comment

patak-dev Mar 12, 2022

Choose a reason for hiding this comment

sapphi-red Mar 13, 2022

Choose a reason for hiding this comment

sapphi-red Mar 13, 2022 • edited

Choose a reason for hiding this comment

sapphi-red Mar 4, 2022

Choose a reason for hiding this comment

dominikg commented Mar 4, 2022

sapphi-red commented Mar 6, 2022

dominikg commented Mar 6, 2022 • edited

patak-dev commented Mar 12, 2022

patak-dev commented Mar 13, 2022

sapphi-red commented Mar 13, 2022 • edited

patak-dev commented Mar 13, 2022

dominikg commented Mar 13, 2022 • edited

patak-dev commented Mar 21, 2022

mbjorke commented Mar 23, 2022

This comment was marked as outdated.

tyouzu1 commented Mar 23, 2022

tyouzu1 commented Mar 23, 2022

sapphi-red commented Mar 4, 2022 •

edited

sapphi-red Mar 13, 2022 •

edited

dominikg commented Mar 6, 2022 •

edited

sapphi-red commented Mar 13, 2022 •

edited

dominikg commented Mar 13, 2022 •

edited