Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS properties ordered alphabetically on build #2395

Closed
lionel-bijaoui opened this issue Aug 29, 2018 · 5 comments
Closed

CSS properties ordered alphabetically on build #2395

lionel-bijaoui opened this issue Aug 29, 2018 · 5 comments

Comments

@lionel-bijaoui
Copy link

lionel-bijaoui commented Aug 29, 2018

Version

3.0.1

Reproduction link

https://github.com/lionel-bijaoui/demo-order-css

Node and OS info

Node 8.11.4 / npm 5.6.0 / Windows 10

Steps to reproduce

Create a vue-cli project with scss support.
Create a basic component with a style block and add a class.
In this class, put the properties in non alphabetical order.
If you run npm run serve and inspect the style, they should be in the same order as what you wrote them.
If you run npm run build and inspect the style, they will be in alphabetical order

What is expected?

.someclass {
    border-radius: 0;
    border-bottom-right-radius: $radius;
    border-top-right-radius: $radius;
}

What is actually happening?

.someclass {
    border-bottom-right-radius: $radius;
    border-radius: 0;
    border-top-right-radius: $radius;
}

The CSS properties get reordered alphabetically and it can create unexpected side effects
The change in behavior was observed between v3.0-beta.10 and 3.0.1 but it might be older.
I guess it has something to do with minification, but I was unable to find any option or mention in the changelog.

EDIT: Added link to repo of demo

@lionel-bijaoui
Copy link
Author

I changed cssDeclarationSorter option of CSSNano to false but it still change the order

@LinusBorg LinusBorg added the needs reproduction This issue is missing a minimal runnable reproduction, provided by the author label Aug 29, 2018
@LinusBorg
Copy link
Member

LinusBorg commented Aug 29, 2018

Please provide a reproduction, especially demonstrating how you tried to change cssDeclarationSorter .

Also: Your code exmaples makes it seem as if the SCSS code would be sorted (it contains variables), but you say the final CSS is sorted. which is it.

@lionel-bijaoui
Copy link
Author

lionel-bijaoui commented Aug 29, 2018

Yes sorry about the variables, it just a bit of code from my project.
The SCSS variable are really not the problem.
Every CSS property get ordered alphabetically. I'm pretty sure you can test it on any of your projects.
It seem to be related to CSSNano (cssnano/cssnano#592, Siilwyn/css-declaration-sorter#55) more than anything.

How?

Let's take my example, I have a class with radius applied on all corner. Then I create another variation (BEM style) with only radius on the right corners.

$radius: 30px;
.some-class {
    border-radius: $radius;
}
.some-class--half-radius-right {
    border-radius: 0;
    border-bottom-right-radius: $radius;
    border-top-right-radius: $radius;
}

In dev mode, no problem, everything work as expected.
But if I build my project, I get that:

.some-class {
    border-radius: 30px;
}
.some-class--half-radius-right {
    border-bottom-right-radius: 30px;
    border-radius: 0;
    border-top-right-radius: 30px;
}

This break the style as border-bottom-right-radius will get overrided by border-radius.
So sure, this example can be fixed easily but it create other kind of problems.
Short-hand declaration can override simple declaration depending on order (or vice versa).
In short it can break the cascade.
When working with legacy code or large project, it become very hard to find and fix.

I just wish to disable this, but I can seem to be able to with cssDeclarationSorter set to false

Thank you for your time

@lionel-bijaoui
Copy link
Author

lionel-bijaoui commented Aug 30, 2018

@LinusBorg As requested, here is a repo that demonstrate the problem:

https://github.com/lionel-bijaoui/demo-order-css

  1. npm run serve, observe the rectangle with radius applied only on the bottom. Inspect it and you will see the properties of .hello--bottom-corner-only are ordered like that:
    .hello--bottom-corner-only {
      border-radius: 0;
      border-bottom-left-radius: 30px;
      border-bottom-right-radius: 30px;
    }
  2. npm run build, open ./dist/css/app.***.css. You will see that the properties are order differently, resulting in no radius on corners:
    .hello--bottom-corner-only[data-v-870f974c] {
      border-bottom-left-radius: 30px;
      border-bottom-right-radius: 30px;
      border-radius: 0;
    }

Hope that help !

EDIT: I pushed a change to set cssDeclarationSorter to false and prove it doesn't change a thing to the issue.
Here is an extract of the config produced by vue-cli-service inspect to show that the option is set correctly:

...
/* config.plugin('optimize-css') */
new OptimizeCssnanoPlugin(
    {
    sourceMap: false,
    cssnanoOptions: {
        safe: true,
        autoprefixer: {
        disable: true
        },
        mergeLonghand: false,
        cssDeclarationSorter: false
    }
    }
),
...

@LinusBorg LinusBorg added needs team repro We acknowledged your report and will soon try to reproduce it and removed needs reproduction This issue is missing a minimal runnable reproduction, provided by the author labels Aug 30, 2018
@sodatea sodatea added bug scope: cli-service build and removed needs team repro We acknowledged your report and will soon try to reproduce it labels Sep 5, 2018
@sodatea
Copy link
Member

sodatea commented Sep 5, 2018

Well, it is a bug in Vue CLI's default configuration, as we have set cssnanoOptions in the wrong way.

Current workaround:

// vue.config.js
module.exports = {
  chainWebpack: config => {
    config.when(process.env.NODE_ENV === "production", config => {
      config.plugin("optimize-css").tap(args => {
        args[0].cssnanoOptions = {
          preset: ["default", {
              autoprefixer: { disable: true },
              mergeLonghand: false,
              cssDeclarationSorter: false
            }
          ]
        };
        return args;
      });
    });
  }
};

@sodatea sodatea closed this as completed in d0320eb Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants