search

postcss / postcss-nested

PostCSS plugin to unwrap nested rules like how Sass does it.
MIT License
1.15k stars 66 forks source link

Change in 4.2.2 affects intended styles #94

Closed paulrobertlloyd closed 4 years ago

paulrobertlloyd commented 4 years ago

With the following source CSS:

.c-meta {
  @apply --typeset-caption;

  display: flex;
  flex-flow: row wrap;
  margin-bottom: map(spaces, medium);
  border-top: 1px solid $prose-color--rule;
  padding: map(spaces, small) 0 map(spaces, large);

  @media (--from-large-screen) {
    flex-flow: column nowrap;
    border: 0;
    padding: 0;
  }
}

.c-meta__item {
  flex: 0 0 auto;

  @media screen and (max-width: 24.9375em) {
    .dt-published span {
      display: none;
    }
  }

  @media (--upto-large-screen) {
    &:not(:last-child)::after {
      content: '•';
      margin: 0 map(spaces, small);
      opacity: 0.5;
    }
  }

  @media (--from-large-screen) {
    display: block;
    border-top: 1px solid $prose-color--rule;
    padding: map(spaces, small) 0 map(spaces, large);
    text-align: right;
  }

  @media print {
    &:not(:last-child)::after {
      content: '•';
      margin: 0 map(spaces, small);
    }
  }
}

The following rules for .c-meta are affected:

  display: flex;
  flex-flow: row wrap;
  margin-bottom: map(spaces, medium);
  border-top: 1px solid $prose-color--rule;
  padding: map(spaces, small) 0 map(spaces, large);

With 4.2.2, these rules – common to .c-meta – are moved out into a separate declaration later in the stylesheet:

.c-meta {
    font-size: calc(.875rem 0.125 *(100vw - 30rem) /30);
    font-family: sans-serif;
    font-weight: 400;
    font-variant: common-ligatures lining-nums;
-     line-height: 1.25;
-     display: flex;
-     flex-flow: row wrap;
-     margin-bottom: 1rem;
-     border-top: 1px solid rgba(119, 119, 136, .25);
-     padding:.5rem 0 1.5rem
+     line-height:1.25
}

@media screen and (min-width: 60rem) {
    .c-meta {
        font-size:1rem
    }
}

@media screen and (max-width: 30rem) {
    .c-meta {
        font-size:.875rem
    }
}

@media print {
    .c-meta {
        font-size:.875rem
    }
}

.fonts-loaded .c-meta {
    font-family:Source Sans Pro, sans-serif
}

@media screen and (min-width: 800px) {
    .c-meta {
        flex-flow: column nowrap;
        border: 0;
        padding:0
    }
}

+ .c-meta {
+     display: flex;
+     flex-flow: row wrap;
+     margin-bottom: 1rem;
+     border-top: 1px solid rgba(119, 119, 136, .25);
+     padding:.5rem 0 1.5rem
+ }

.c-meta__item {
    flex:0 0 auto
}

@media screen and (max-width: 24.9375em) {
    .c-meta__item .dt-published span {
        display:none
    }
}

@media screen and (max-width: 799px) {
    .c-meta__item:not(:last-child):after {
        content: "•";
        margin: 0 .5rem;
        opacity:.5
    }
}

@media screen and (min-width: 800px) {
    .c-meta__item {
        display: block;
        border-top: 1px solid rgba(119, 119, 136, .25);
        padding: .5rem 0 1.5rem;
        text-align:right
    }
}

@media print {
    .c-meta__item:not(:last-child):after {
        content: "•";
        margin:0 .5rem
    }
}

It’s new location means this portion of CSS is overruled:

@media screen and (min-width: 800px) {
    .c-meta {
        flex-flow: column nowrap;
        border: 0;
        padding:0
    }
}

I suspect this is related to the change in #93. Let me know if you need more information.

ai commented 4 years ago

Can you add console.log(root.toString) to plugin sources to show the plugin input, not a sources (syntax constructions from other plugins make it easy to understand what happens here).

ai commented 4 years ago

@paulrobertlloyd will this PR fix your issue? https://github.com/postcss/postcss-nested/pull/95

You can test it by npm i postcss-nested@demikhovr/postcss-nested#fix-for-declarations-before-at-rule

paulrobertlloyd commented 4 years ago

95 doesn’t appear to resolve this issue.

Can you tell me where I should add console.log(root.toString)? I’m using gulp-postcss. Here’s my task:

const postcss = require('gulp-postcss');

...

function styles() {
  return gulp.src(`${paths.src}/assets/styles/*.css`)
    .pipe(sourcemaps.init())
    .pipe(postcss(processors))
    .pipe(sourcemaps.write('./'))
    .pipe(gulp.dest(`${paths.dest}/assets/styles`));
}

processors is an array of PostCSS plugins:

const processors = [
  importer({
    glob: true
  }),
  mapper({
    maps: [
      `${paths.src}/tokens/borders.json`,
      `${paths.src}/tokens/breakpoints.json`,
      `${paths.src}/tokens/colors.json`,
      `${paths.src}/tokens/fonts.json`,
      `${paths.src}/tokens/layers.json`,
      `${paths.src}/tokens/sizes.json`,
      `${paths.src}/tokens/spaces.json`
    ]
  }),
  assets({
    loadPaths: [`${paths.src}/assets/vectors`]
  }),
  simpleVars,
  apply,
  calc,
  customMedia,
  colorFunction,
  mediaMinMax,
  nested,
  responsiveType,
  autoprefixer,
  nano
];

(I found this bug on an old project, which in retrospect is needlessly complex, so forgive my setup).

ai commented 4 years ago

Can you tell me where I should add console.log(root.toString)?

Open node_modules/postcss-nested/index.js and change line 201:

  }
- return process
+ return root => {
+   console.log(root.toString())
+   process(root)
+ }
})
paulrobertlloyd commented 4 years ago

Here you go:

.c-meta {
  font-size: responsive 0.875rem 1rem;
  font-range: 30rem 60rem;
  font-family: sans-serif;
  font-weight: 400;
  font-variant: common-ligatures lining-nums;
  line-height: 1.25;

  @media print {
    font-size: 0.875rem;
  }

  .fonts-loaded & {
    font-family: 'Source Sans Pro', sans-serif;
  }

  display: flex;
  flex-flow: row wrap;
  margin-bottom: 1rem;
  border-top: 1px solid rgba(119, 119, 136, 0.25);
  padding: 0.5rem 0 1.5rem;

  @media screen and (min-width: 800px) {
    flex-flow: column nowrap;
    border: 0;
    padding: 0;
  }
}

.c-meta__item {
  flex: 0 0 auto;

  @media screen and (max-width: 24.9375em) {
    .dt-published span {
      display: none;
    }
  }

  @media screen and (max-width: 799px) {
    &:not(:last-child)::after {
      content: '•';
      margin: 0 0.5rem;
      opacity: 0.5;
    }
  }

  @media screen and (min-width: 800px) {
    display: block;
    border-top: 1px solid rgba(119, 119, 136, 0.25);
    padding: 0.5rem 0 1.5rem;
    text-align: right;
  }

  @media print {
    &:not(:last-child)::after {
      content: '•';
      margin: 0 0.5rem;
    }
  }
}
demikhovr commented 4 years ago

My bad most likely 🙁 I reproduced the issue with your example, @paulrobertlloyd. The fix in #95 should place this separate rule

.c-meta {
   display: flex;
   flex-flow: row wrap;
   margin-bottom: 1rem;
   border-top: 1px solid rgba(119, 119, 136, .25);
   padding:.5rem 0 1.5rem;
}

right before this

@media screen and (min-width: 800px) {
    .c-meta {
        ...
    }
}

and the rule won't be overruled.

ai commented 4 years ago

@paulrobertlloyd I released @demikhovr fix in 4.2.3. Can you try?

paulrobertlloyd commented 4 years ago

The good news: the regression described in my original issue has been fixed. 🎉

The bad news: there was a second, less noticeable regression, that I can confirm still exists with 4.2.3.

Here’s the diff:

.c-og-image {
    width: 600px;
    height: 315px;
    overflow: hidden;
    padding: 2rem;
    background-color: #520010;
    background-color: var(--color-year, #520010);
    background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='136' height='136'%3E%3Cg stroke='%23000' stroke-opacity='.25' stroke-width='2' fill='none'%3E%3Cpath d='M-66.35 68L68-66.35 202.35 68 68 202.35z'/%3E%3Cpath d='M-60.693 68L68-60.693 196.693 68 68 196.693z'/%3E%3Cpath d='M-55.037 68L68-55.037 191.037 68 68 191.037z'/%3E%3Cpath d='M-49.38 68L68-49.38 185.38 68 68 185.38z'/%3E%3Cpath d='M-43.723 68L68-43.723 179.723 68 68 179.723z'/%3E%3Cpath d='M-38.066 68L68-38.066 174.066 68 68 174.066z'/%3E%3Cpath d='M-32.41 68L68-32.41 168.41 68 68 168.41z'/%3E%3Cpath d='M-26.752 68L68-26.752 162.752 68 68 162.752z'/%3E%3Cpath d='M-21.095 68L68-21.095 157.095 68 68 157.095z'/%3E%3Cpath d='M-15.439 68L68-15.439 151.439 68 68 151.439z'/%3E%3Cpath d='M-9.782 68L68-9.782 145.782 68 68 145.782z'/%3E%3Cpath d='M-4.125 68L68-4.125 140.125 68 68 140.125z'/%3E%3Cpath d='M1.532 68L68 1.532 134.468 68 68 134.468z'/%3E%3Cpath d='M7.189 68L68 7.189 128.811 68 68 128.811z'/%3E%3Cpath d='M12.846 68L68 12.846 123.154 68 68 123.154z'/%3E%3Cpath d='M18.503 68L68 18.503 117.497 68 68 117.497z'/%3E%3Cpath d='M24.16 68L68 24.16 111.84 68 68 111.84z'/%3E%3Cpath d='M29.816 68L68 29.816 106.184 68 68 106.184z'/%3E%3Cpath d='M35.473 68L68 35.473 100.527 68 68 100.527z'/%3E%3Cpath d='M41.13 68L68 41.13 94.87 68 68 94.87z'/%3E%3Cpath d='M46.787 68L68 46.787 89.213 68 68 89.213z'/%3E%3Cpath d='M52.444 68L68 52.444 83.556 68 68 83.556z'/%3E%3Cpath d='M58.1 68l9.9-9.9 9.9 9.9-9.9 9.9z'/%3E%3Cpath d='M63.757 68L68 63.757 72.243 68 68 72.243z'/%3E%3C/g%3E%3C/svg%3E");
    background-repeat:repeat
}

.c-og-image__quote {
    font-family: serif;
    font-variant: common-ligatures oldstyle-nums;
    line-height: 1.5;
    letter-spacing: -.0125em;
    -webkit-hyphens: auto;
    hyphens: auto;
-   hanging-punctuation: first;
+   hanging-punctuation:first
+ }
+
+ .c-og-image__quote, .fonts-loaded .c-og-image__quote {
+   font-family: "Source Serif Pro", serif
+ }
+
+ .c-og-image__quote {
    margin-bottom: .5rem;
    font-size: 2rem;
    line-height: 1.15;
    text-indent: -.45em;
    color:#fff
}

- .c-og-image__quote, .fonts-loaded .c-og-image__quote {
-     font-family: "Source Serif Pro", serif
- }
-
@supports (hanging-punctuation: first) {
    .c-og-image__quote {
        text-indent:0
    }
}

.c-og-image__quote--small {
    font-size:1.75rem
}

.c-og-image__cite {
    font-size: calc(.875rem + 0.125 *(100vw - 30rem) /30);
    font-family: sans-serif;
    font-weight: 400;
    font-variant: common-ligatures lining-nums;
-   line-height: 1.25;
-   font-weight: 600;
-   font-size: 1.25rem;
-   color:#f04
+   line-height:1.25
}

@media screen and (min-width: 60rem) {
    .c-og-image__cite {
        font-size:1rem
    }
}

@media screen and (max-width: 30rem) {
    .c-og-image__cite {
        font-size:.875rem
    }
}

@media print {
    .c-og-image__cite {
        font-size:.875rem
    }
}

.fonts-loaded .c-og-image__cite {
    font-family:Source Sans Pro, sans-serif
}

+ .c-og-image__cite {
+   font-weight: 600;
+   font-size: 1.25rem;
+   color:#f04
+ }
+
.c-og-image .c-avatar {
    float: right;
    margin:-2.5rem -3.5rem 0 0
}

Note how .c-og-image__cite comes after the media queries, thus overruling the change in font size?

Here’s the source using 4.2.3 (from the debugging method described before):

.c-og-image {
  width: 600px;
  height: 315px;
  overflow: hidden;
  padding: 2rem;
  background-color: hsl(348, 100%, 16%);
  background-color: var(--color-year, hsl(348, 100%, 16%));
  background-image: url('data:image/svg+xml;charset=utf-8,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20width%3D%22136%22%20height%3D%22136%22%3E%0A%20%20%3Cg%20transform%3D%22translate(68%2C%2068)%20rotate(-45)%20translate(-68%2C%20-68)%20translate(-27%2C%20-27)%22%20stroke%3D%22%23000%22%20stroke-opacity%3D%220.25%22%20stroke-width%3D%222%22%20fill%3D%22none%22%3E%0A%20%20%20%20%3Crect%20x%3D%220%22%20y%3D%220%22%20width%3D%22190%22%20height%3D%22190%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%224%22%20y%3D%224%22%20width%3D%22182%22%20height%3D%22182%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%228%22%20y%3D%228%22%20width%3D%22174%22%20height%3D%22174%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2212%22%20y%3D%2212%22%20width%3D%22166%22%20height%3D%22166%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2216%22%20y%3D%2216%22%20width%3D%22158%22%20height%3D%22158%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2220%22%20y%3D%2220%22%20width%3D%22150%22%20height%3D%22150%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2224%22%20y%3D%2224%22%20width%3D%22142%22%20height%3D%22142%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2228%22%20y%3D%2228%22%20width%3D%22134%22%20height%3D%22134%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2232%22%20y%3D%2232%22%20width%3D%22126%22%20height%3D%22126%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2236%22%20y%3D%2236%22%20width%3D%22118%22%20height%3D%22118%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2240%22%20y%3D%2240%22%20width%3D%22110%22%20height%3D%22110%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2244%22%20y%3D%2244%22%20width%3D%22102%22%20height%3D%22102%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2248%22%20y%3D%2248%22%20width%3D%2294%22%20height%3D%2294%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2252%22%20y%3D%2252%22%20width%3D%2286%22%20height%3D%2286%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2256%22%20y%3D%2256%22%20width%3D%2278%22%20height%3D%2278%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2260%22%20y%3D%2260%22%20width%3D%2270%22%20height%3D%2270%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2264%22%20y%3D%2264%22%20width%3D%2262%22%20height%3D%2262%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2268%22%20y%3D%2268%22%20width%3D%2254%22%20height%3D%2254%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2272%22%20y%3D%2272%22%20width%3D%2246%22%20height%3D%2246%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2276%22%20y%3D%2276%22%20width%3D%2238%22%20height%3D%2238%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2280%22%20y%3D%2280%22%20width%3D%2230%22%20height%3D%2230%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2284%22%20y%3D%2284%22%20width%3D%2222%22%20height%3D%2222%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2288%22%20y%3D%2288%22%20width%3D%2214%22%20height%3D%2214%22%2F%3E%0A%20%20%20%20%3Crect%20x%3D%2292%22%20y%3D%2292%22%20width%3D%226%22%20height%3D%226%22%2F%3E%0A%20%20%3C%2Fg%3E%0A%3C%2Fsvg%3E');
  background-repeat: repeat;
}

.c-og-image__quote {
  font-family: serif;
  font-variant: common-ligatures oldstyle-nums;
  line-height: 1.5;
  letter-spacing: -0.0125em;
  hyphens: auto;
  hanging-punctuation: first;

    .fonts-loaded & {
  font-family: 'Source Serif Pro', serif;
    }

  margin-bottom: 0.5rem;
  font-family: 'Source Serif Pro', serif;
  font-size: 2rem;
  line-height: 1.15;
  text-indent: -0.45em;
  color: white;
}

@supports (hanging-punctuation: first) {
  .c-og-image__quote {
    text-indent: 0;
  }
}

.c-og-image__quote--small {
  font-size: 1.75rem;
}

.c-og-image__cite {
  font-size: responsive 0.875rem 1rem;
  font-range: 30rem 60rem;
  font-family: sans-serif;
  font-weight: 400;
  font-variant: common-ligatures lining-nums;
  line-height: 1.25;

    @media print {
  font-size: 0.875rem;
    }

    .fonts-loaded & {
  font-family: 'Source Sans Pro', sans-serif;
    }

  font-weight: 600;
  font-size: 1.25rem;
  color: #f04;
}

.c-og-image .c-avatar {
  float: right;
  margin: -2.5rem -3.5rem 0 0;
}

Again, apologies for the complexity, but I guess it’s a good test of recent changes to this module. Were I writing this CSS today, I’d no doubt use fewer PostCSS plugins and media queries!

demikhovr commented 4 years ago

Yes, it's predictable behaviour. In #93(+#95) PR changes were added to fix issue #92. Now declarations after nested rules based on the order in which declarations were originally defined.

.c-og-image__cite comes after .fonts-loaded .c-og-image__cite { ... } since

.c-og-image__cite {
  ...

  .fonts-loaded & {
    font-family: 'Source Sans Pro', sans-serif;
  }

  // following declarations come after nested rule
  font-weight: 600;
  font-size: 1.25rem;
  color: #f04;
}
paulrobertlloyd commented 4 years ago

Fair enough. Thanks for co-ordinating the earlier fix!