10K+ lines of redundant CSS delivered on every project page

scratchfoundation / scratch-www

Standalone web client for Scratch

https://scratch.mit.edu

BSD 3-Clause "New" or "Revised" License

1.59k stars 845 forks source link

10K+ lines of redundant CSS delivered on every project page #4783

Open astrosticks opened 3 years ago

astrosticks commented 3 years ago

Steps to Reproduce

Open a project page
Inspect page, view inner html of <head>
Observe over 15,000 lines of code in <style> tags, most of it redundant copies of itself

This behavior is consistent on a project with many comments and a project with very few comments.

Operating System and Browser

Firefox 78.6.0esr on Debian 10

GrahamSH-LLK commented 3 years ago

It looks to me that they aren't copies, each has similar comments, but different css.

mxmou commented 3 years ago

It looks to me that they aren't copies, each has similar comments, but different css.

Most stylesheets @import _colors.scss and _frameless.scss, so they're actually copies.

benjiwheeler commented 3 years ago

I'm not marking this help wanted, because solving it may be complex. But if anyone has suggestions, we're interested in hearing them!

Note that it's not that the full css is redundant, just that we do this all over the place:

@import "../../colors";
@import "../../frameless";

maybe there's a way to only do this once?

GrahamSH-LLK commented 3 years ago

https://stackoverflow.com/a/25251042 may be helpful. What if we had a main.scss for each page, which imported necessary files? cc @benjiwheeler

benjiwheeler commented 3 years ago

@GrahamSH-LLK Thanks for the tip -- I don't think that approach will work because we're including scss files in their corresponding components; they need to individually compile, so they can't wait for the main.scss (which we do have for every page already) to provide values for the SASS variables.

Per this discussion: https://github.com/webpack-contrib/sass-loader/issues/145#issuecomment-173917670 , maybe gzip means we don't need to worry about this duplication?

GrahamSH-LLK commented 3 years ago

@GrahamSH-LLK Thanks for the tip -- I don't think that approach will work because we're including scss files in their corresponding components; they need to individually compile, so they can't wait for the main.scss (which we do have for every page already) to provide values for the SASS variables.

Per this discussion: webpack-contrib/sass-loader#145 (comment) , maybe gzip means we don't need to worry about this duplication?

Not AFAIK, because the CSS is dynamically loaded on the page (correct me if I'm wrong.)

apple502j commented 3 years ago

splash: Transferred 1.93 KB (3.69 KB size) projects: Transferred 1.45 KB (1.92 KB size)

in terms of data transport, gzip should take care of this.

Question: can we use CSS variables? Supported in Edge 16, Safari 10, Firefox 31 and Chrome 49. IE does not support it, but it may be a good time to drop support for old browsers (of course after research.)

Accio1 commented 3 years ago

splash: Transferred 1.93 KB (3.69 KB size) projects: Transferred 1.45 KB (1.92 KB size)

in terms of data transport, gzip should take care of this.

Question: can we use CSS variables? Supported in Edge 16, Safari 10, Firefox 31 and Chrome 49. IE does not support it, but it may be a good time to drop support for old browsers (of course after research.)

@apple502j according to the FAQ Page, Internet Explorer is already not supported.

GrahamSH-LLK commented 3 years ago

What if we remove all comments and css from the imported files? We wouldn't have any duplicated css, because variables shouldn't output to actual css.

mxmou commented 3 years ago

Related: #442