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.

Sign up for GitHub

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

Jump to bottom

Vite lossily combines link[rel=stylesheet] and link[rel=stylesheet][media="(prefers-color-scheme:dark)"] #6748

Closed
7 tasks done
tomayac opened this issue Feb 3, 2022 · 5 comments · Fixed by #6751
Closed
7 tasks done

Vite lossily combines link[rel=stylesheet] and link[rel=stylesheet][media="(prefers-color-scheme:dark)"] #6748

tomayac opened this issue Feb 3, 2022 · 5 comments · Fixed by #6751
Labels
feat: css p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@tomayac
Copy link
Contributor

tomayac commented Feb 3, 2022

Describe the bug

Vite lossily (note the media attribute) combines

<link rel=stylesheet href="a.css" />
<link rel=stylesheet href="b.css" media="(prefers-color-scheme:dark)" />

to

<link rel=stylesheet href="c123.css" />

How to prevent this? Ideally both files were optimized and name-hashed, but not combined into one and the media attribute preserved:

<link rel=stylesheet href="a123.css" />
<link rel=stylesheet href="b456.css" media="(prefers-color-scheme:dark)" />

Reproduction

https://stackblitz.com/edit/vitejs-vite-tbytgv?devtoolsheight=33&file=index.html

System Info

N/A

Used Package Manager

npm

Logs

❯ npm run build
$ vite build
vite v2.7.13 building for production...
✓ 3 modules transformed.
dist/assets/favicon.17e50649.svg   1.49 KiB
dist/index.html                    0.35 KiB
dist/assets/index.3d17f766.css     0.05 KiB / gzip: 0.05 KiB

Validations
@Niputi Niputi added feat: css p3-minor-bug An edge case that only affects very specific usage (priority) bug labels Feb 3, 2022
pd4d10 added a commit to pd4d10/vite that referenced this issue Feb 4, 2022
@pd4d10
fix: link tag with media prop
8c5f3bc 
closes vitejs#6748
pd4d10 added a commit to pd4d10/vite that referenced this issue Feb 4, 2022
@pd4d10
fix: link tag with media prop
bae30d7 
closes vitejs#6748
@pd4d10 pd4d10 mentioned this issue Feb 4, 2022
Merged
9 tasks
@pd4d10
Copy link
Contributor

pd4d10 commented Feb 4, 2022

Besides media, are there any other attributes that would affect combine strategy?

Maybe we can deal with it together in this PR #6751

@tomayac
Copy link
Contributor Author

tomayac commented Feb 4, 2022

Besides media, are there any other attributes that would affect combine strategy?

Maybe disabled. I have seen folks use this to switch stylesheets. My expectation for Vite would be to also not bundle disabled stylesheets, but to process them separately.

@antfu antfu mentioned this issue May 17, 2022
Closed
7 tasks
@Lastor-Chen
Copy link

Lastor-Chen commented May 17, 2022 •
edited

Hello, I have same issues for this.
I want to split scss files into mobile and desktop for mobile optimization. It's working on serve mode. (css is the same)

<link rel="stylesheet" href="/src/styles/mobile.scss">
<link rel="stylesheet" href="/src/styles/desktop.scss" media="(min-width: 768px)">

But vite combie these on build and lost the media attribute.

<link rel="stylesheet" href="/assets/style.c5411e4b.css">

I tried to set rollup options like this.

// vite.config
export default defineConfig({
  return {
    build: {
      rollupOptions: {
        output: {
          manualChunks(id) {
            if (id.includes('.scss')) {
              // e.g. ".../styles/desktop.scss" to "desktop"
              const cssName = id.split('styles/')[1].split('.')[0]
              return cssName
            }
          },
        },
      },
    },
  }
})

vite split the css files as expected. but still could not save the media attribute. So I manually add the media attribute after build.

// build output
<link rel="stylesheet" href="/assets/mobile.6f120d68.css">
<link rel="stylesheet" href="/assets/desktop.76768932.css">

I saw PR #6751, will this feature be released in the next version?

Edit

Well, I thought link media would allow browser to automatically choose which one to download. But it doesn't seem to work that way. I got it wrong.

@tomayac tomayac mentioned this issue May 27, 2022
Closed
@surma
Copy link

surma commented Jul 20, 2022

I ran into this issue, because I provide print styles and screen styles:

<!doctype html>
<link rel="stylesheet" href="/screen.css" media="screen" />
<link rel="stylesheet" href="/print.css" media="print" />

Only ever one of these files should load. But vite combines them into one, breaking my site a bit 😅

@tomayac
Copy link
Contributor Author

tomayac commented Jul 25, 2022

It looks like #6751 is ready to go. Thanks to everyone involved for working on this! <3

@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
feat: css p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: support stylesheets with link tag and media/disable prop pd4d10/vite
5 participants
@tomayac @surma @Niputi @pd4d10 @Lastor-Chen