SassNinja / media-query-plugin

Webpack plugin for media query extraction.
MIT License
205 stars 27 forks source link

Perhaps dynamic import is useless when we already have media query? #19

Open joe223 opened 5 years ago

joe223 commented 5 years ago

In this example example/webpack,we use

import(/* webpackChunkName: 'example-desktop' */ './example-desktop.scss');

to import an example-desktop file dynamically.

But in the result file,media-query code :

<link rel="stylesheet" href="example2-desktop.css" media="print, screen and (max-width: 60em)"

will load css file already and automatically.

So,Is examples/webpack/src/example-desktop.scss file and javascript dynamic import necessary?

BTW,what you're doing is wonderful! Thanks

joe223 commented 5 years ago

Oh, I have forgot that this demo inject assets manually, https://github.com/SassNinja/media-query-plugin/blob/13012c0bc18568bdc8453c6afd92c3e2370a4e28/examples/webpack/src/index.hbs#L5 https://github.com/SassNinja/media-query-plugin/blob/13012c0bc18568bdc8453c6afd92c3e2370a4e28/examples/webpack/src/index.hbs#L9 But I did not figure out where is htmlWebpackPlugin come from?

joe223 commented 5 years ago

https://github.com/jantimon/html-webpack-plugin/blob/35a154186501fba3ecddb819b6f632556d37a58f/index.js#L62 I got,userOption will overwrite templateParams 😂

joe223 commented 5 years ago

From htmlWebpackPlugin assets info:

"chunks": {
        "vendors": {
            "size": 271261,
            "entry": "/vendors.53892e9fcf73842414ca.js",
            "hash": "d3b3e4f391f4d99c53e2",
            "css": []
        },
        "contact-us": {
            "size": 5866,
            "entry": "/contact-us.53892e9fcf73842414ca.js",
            "hash": "14a775d1f787fce8fffc",
            "css": [
                "/contact-us.css"
            ]
        }
    },
    "js": [
        "/vendors.53892e9fcf73842414ca.js",
        "/contact-us.53892e9fcf73842414ca.js"
    ],
    "css": [
        "/contact-us.css"
    ],
  "extracted": {
        "css": [
            {
                "file": "about_us-desktop.css",
                "query": "screen and (min-width: 769px)"
            },
            {
                "file": "contact_us-desktop.css",
                "query": "screen and (min-width: 769px)"
            },
            {
                "file": "index-desktop.css",
                "query": "screen and (min-width: 769px)"
            }
        ]
    }

We have no idea of *-desktop.css's entry,It will cause extra resource loading in multiple entry. Maybe we can add some additional info to indicate or loading assets automatically?

Josecc commented 5 years ago

I had the same thought @joe223 but it looks like all browsers download all stylesheets regardless of whether they match the media property, and will only apply them when it matches. eg: https://scottjehl.github.io/CSS-Download-Tests/

joe223 commented 5 years ago

Yes, it is. 😂

So I am thinking how can I load style files automatically...

felixfbecker commented 4 years ago

I'd really like to use the plugin without having to create empty dummy files and dynamic imports, I just want MiniCssExtractPlugin to output 2 files, one for (prefers-color-scheme: light) and one for (prefers-color-scheme: dark). I'd much prefer to load these through a <link> tag, because doing it with a dynamic import means the JS bundle has to load and execute first.

SassNinja commented 4 years ago

Hi everyone,

sorry for the late response

I've noticed my webpack example was erroneous and as a result all stylesheets got downloaded without respecting the current viewport. Therefore I've worked over the example to demonstrate a reasonable case (example-desktop.css doesn't get downloaded on mobile viewport anymore now) https://github.com/SassNinja/media-query-plugin/tree/master/examples/webpack

I had the same thought @joe223 but it looks like all browsers download all stylesheets regardless of whether they match the media property, and will only apply them when it matches.

@Josecc that's right, the media attribute is only about applying the styles, not about downloading the file (my original intention was just to demonstrate this, that's why I called it example2)

So I am thinking how can I load style files automatically...

@joe223 unfortunately that's not possible out-of-the-box at the moment because you've to do the dynamic imports yourself if you don't want the client to download unnecessary CSS.

Take a look at my reworked example to get an idea how things may be built. Though it's just a basic example which can be improved for sure (e.g. by not using handlebars what I start to hate more and more)

I'd really like to use the plugin without having to create empty dummy files and dynamic imports, I just want MiniCssExtractPlugin to output 2 files, one for (prefers-color-scheme: light) and one for (prefers-color-scheme: dark). I'd much prefer to load these through a tag, because doing it with a dynamic import means the JS bundle has to load and execute first.

@felixfbecker I understand it's not convenient to import empty files just to get populated by the plugin. Unfortunately I've not found a better way yet to integrate the emitted files into the bundle (so hashed files are possible).

Regarding not to use dynamic imports: If you don't use them there's no way to prevent your users from unnecessary CSS files. As mentioned above the media attribute doesn't prevent the download.

IF your concern is related to FOUC I'd recommend to not extract your critical CSS.

felixfbecker commented 4 years ago

@felixfbecker I understand it's not convenient to import empty files just to get populated by the plugin. Unfortunately I've not found a better way yet to integrate the emitted files into the bundle (so hashed files are possible).

I wonder how the chunks plugin does it, since it also at a high level splits one input file into multiple? Could the same approach be used?

Regarding not to use dynamic imports: If you don't use them there's no way to prevent your users from unnecessary CSS files. As mentioned above the media attribute doesn't prevent the download.

IF your concern is related to FOUC I'd recommend to not extract your critical CSS.

In my use case downloading the extra stylesheet is generally not a concern, because the other stylesheet may be needed by the user down the line anyway, e.g. if they switches their theme, or in the case of size media queries, resize their browser window. The negative effect of that is much smaller than the negative effect of having to wait until a (in comparison) massive JS bundle is downloaded, parsed, executed, then starting to download the stylesheet.

There often isn't a clear distinction between "critical" and non-critical CSS. In the case of dark vs light theme for example, those theme-specific styles are all critical because you don't want your site to intermediately be visible with the wrong colors. That has a higher importance than not downloading another stylesheet in the background.

Could this maybe be an option of the plugin, to support this approach for folks whose constraints match this?

SassNinja commented 4 years ago

@felixfbecker Could this maybe be an option of the plugin, to support this approach for folks whose constraints match this?

I know it's been a while ago but would you describe that desired option in detail?

Apart from creating extracted chunks for dynamic imports (what you said is not relevant for you) the plugin already supports what you need.