postcss / postcss-nested

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

Declarations in wrong specificity order #92

Closed jednano closed 4 years ago

jednano commented 4 years ago

Steps to reproduce

Run postcss-cli with the following command:

postcss test.css -u postcss-nested

Given the following input from test.css:

.a {
  .b {
    c: d;
    &e {
      f: g
    }
    h: i
  }
}

I am given the following output:

.a .b {
    c: d;
    h: i;
  }
    .a .be {
      f: g
    }

Issues

  1. Both rules .a .b and .a .be have the same specificity, so the h: i declaration should come at the very end in a new .a .b rule, based on the order in which declarations were originally defined (not how Sass does it, but more accurate).
  2. ~The h: i declaration was given a semi-colon that it didn't previously have. This might be an issue with the structure of the AST raws, as the rule defines the semi-colon and not the declaration?~
  3. ~All rule indentation should be at the root of the document (definitely how Sass does it).~
  4. ~Sending the AST directly through postcss-nested doesn't completely unwrap the rules. I have to call the plugin multiple times to get complete unwrapping. If I have rules nested 4 levels deep, I have to call the plugin 4x!~

For the last issue, this is what I was doing from within a custom plugin:

const nested = postcssNested();
return (root, result) => {
  // do stuff
  nested(root, result);
};

Also, hi @ai! 👋 Long time, no see!

ai commented 4 years ago
  1. Both rules .a .b and .a .be have the same specificity, so the h: i declaration should come at the very end in a new .a .b rule, based on the order in which declarations were originally defined (not how Sass does it, but more accurate).

Yeap, let’s fix it. Can you send PR, because I am working on another project right now?

  1. The h: i declaration was given a semi-colon that it didn't previously have.
  2. All rule indentation should be at the root of the document (definitely how Sass does it).

I decided that postcss-nested will not take care bout good formation. It is hard and most of the users will even not see this.

It could increase code 2x-3x times, especially if we will take care of all use cases.

  1. Sending the AST directly through postcss-nested doesn't completely unwrap the rules. I have to call the plugin multiple times to get complete unwrapping. If I have rules nested 4 levels deep, I have to call the plugin 4x!

It is very strange because PostCSS itself does exactly the same nested(root, result) call.

Can you create a simple test stand to reproduce the issue?

Also, hi @ai! wave Long time, no see!

Nice to see you again =^_^=. I started to use TypeScript in some of my projects. Also, I am planning to visit Austin in August.

jednano commented 4 years ago
  1. I can't, actually.
  2. In hindsight, I agree.
  3. Agreed.
  4. I figured out that the culprit was my plugin when I converted at-rules into rules w/o using newRule.append. Turns out you also can't loop through container.nodes, which is what I was doing as well. Would love a generator method so I can do for (const node of rule.nodes()), because then I can continue and break and even yield if I wanted to.

I sent you an email with my phone number for when you come to Austin 😃

ai commented 4 years ago

@jedmao I created a task https://cultofmartians.com/tasks/postcss-nested.html#task

jednano commented 4 years ago

Excellent! Thanks for doing that!

From: Andrey Sitnikmailto:notifications@github.com Sent: Saturday, July 4, 2020 1:40 AM To: postcss/postcss-nestedmailto:postcss-nested@noreply.github.com Cc: Jed Maomailto:jedmao@outlook.com; Mentionmailto:mention@noreply.github.com Subject: Re: [postcss/postcss-nested] Declarations in wrong specificity order (#92)

@jedmaohttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjedmao&data=02%7C01%7C%7Cf9e1de16731b4bf1c63508d81fe53598%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637294416586667667&sdata=sajCU235U05n566QQDhLXyVPDdIUeC70cn4HTG2RsTk%3D&reserved=0 I created a task https://cultofmartians.com/tasks/postcss-nested.html#taskhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcultofmartians.com%2Ftasks%2Fpostcss-nested.html%23task&data=02%7C01%7C%7Cf9e1de16731b4bf1c63508d81fe53598%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637294416586677665&sdata=FnDtwPO3YpeYKo6OvSne5GM8Su4MiJ2KQ2McHGC%2Fk0k%3D&reserved=0

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostcss%2Fpostcss-nested%2Fissues%2F92%23issuecomment-653729398&data=02%7C01%7C%7Cf9e1de16731b4bf1c63508d81fe53598%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637294416586687659&sdata=4uJ9lbrb4CilUiueMnH7aD6zyDuOh1CiDmKiISONBqA%3D&reserved=0, or unsubscribehttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAICLQ7BIML32RSJLIMGUSLRZ3FHRANCNFSM4NWKA3OA&data=02%7C01%7C%7Cf9e1de16731b4bf1c63508d81fe53598%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637294416586697653&sdata=v6Q8it%2BXMeecOw8kFOxpurF5ghkvX6KhjRnMWmFh%2BiY%3D&reserved=0.

demikhovr commented 4 years ago

Hi, I created PR https://github.com/postcss/postcss-nested/pull/93

ai commented 4 years ago

The fix by @demikhovr was released in 4.2.2