Netlify deploy preview

https://deploy-preview-42064--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 13096fa

Seems this change is breaking something as is evident in the argos screenshot. I'll investigate and see if we need a deeper level of testing before merging it.

@brijeshb42 margins for Stack's children elements in the smallest breakpoint seem to be injected last, so they win. This is from the Responsive values example in System's docs:

before	after

After testing the change, I feel that although this is the correct change to have and will benefit us in Pigment CSS to generate relatively smaller bundle sizes, I'm afraid that we might be breaking sites for a lot of our users who might have unknowingly started to rely on this behaviour. Here's an example from one of our own templates that got captured through argos -

Before	After
https://next.mui.com/material-ui/getting-started/templates/dashboard/	https://deploy-preview-42064--material-ui.netlify.app/material-ui/getting-started/templates/dashboard/

cc: @siriwatknp

@brijeshb42 I see the problem now. Before this change, users' overrides always came after the component code.

The original code does this (in a minimal pseudo-code representation):

.Toolbar {
  padding-left: 16px;
}

@media (min-width: 600px) {
  .Toolbar {
    padding-left: 24px;
  }
}

/* User code 👇 */
@media (min-width: 0px) {
  .Toolbar {
    padding-left: 8px; /* 🏆 this declaration wins 🟢 */
  }
}

With the new changes, the generated code looks like this:

.Toolbar {
  padding-left: 16px;
  
  /* User code is mixed with the component's CSS 👇 */
  padding-left: 8px;
}

@media (min-width: 600px) {
  .Toolbar {
    padding-left: 24px; /* 🏆 this declaration wins 🔴 */
  }
}

I'm wondering if the solution is to follow the same approach as the original code by appending a new ruleset at the end but without the media query:

.Toolbar {
  padding-left: 16px;
}

@media (min-width: 600px) {
  .Toolbar {
    padding-left: 24px;
  }
}

/* User code 👇 */
.Toolbar {
  padding-left: 8px; /* 🏆 this declaration would win 🟢 */
}

I'm unaware if this is possible or not, I'm just wondering if this could lead to a potential solution.

It is definitely possible. But in that case, the only gain I see that we are saving this much string (@media (min-width: 0px) {}) from being added which isn't a lot I'd say.

the only gain I see that we are saving this much string (@media (min-width: 0px) {}) from being added which isn't a lot I'd say.

Yeah, although how many times it's repeated in the generated CSS bundle depends on consumers' code. Which may not be so bad when compression enters the equation. But if we think about the big picture (thousands of CSS files being generated by Pigment CSS in the future), may be worth saving some bytes if the added complexity and risk is low.

I did a basic gzipped size check for this template

First one with zero media query -

fs = require('fs');

a = '';
for (let i=0;i<10000;i++) {
 a += `@media (min-width: 0px){.class${i}{padding:10px;}}` + `@media (min-width: ${i}px){.cls${i}{margin: ${i}px;}}`
}

fs.writeFileSync('a1.css', a, 'utf-8')

And second one without media query -

fs = require('fs');

a = '';
for (let i=0;i<10000;i++) {
 a += `.class${i}{padding:10px;}` + `@media (min-width: ${i}px){.cls${i}{margin: ${i}px;}}`
}

fs.writeFileSync('a2.css', a, 'utf-8')

There's definitely reduced size without it (~7.4%)
Before:

After:

@aarongarciah I spoke too soon. I tried to implement what you suggested -

.Toolbar {
  padding-left: 16px;
}

@media (min-width: 600px) {
  .Toolbar {
    padding-left: 24px;
  }
}

/* User code 👇 */
.Toolbar {
  padding-left: 8px; /* 🏆 this declaration would win 🟢 */
}

but there doesn't seem to be a straightforward way to do this as the modification will need to be down outside of the media queries object.