Skip to content

Conversation

@RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Jan 3, 2022

This PR will improve the collapsing of duplicate declarations logic.

In theory, we don't have to do anything because the browser is smart enough to figure everything out. However, leaving in duplicate properties is not that ideal for file size.

Our previous method was pretty simple: if you see a declaration you already saw in this rule, delete the previous one and keep the current one.

This works pretty well, but this gets rid of all the declarations with the same property. This is not great for overrides for older browsers.

In a perfect world, we can handle this based on your target browser but this is a lot of unnecessary complexity and will slow things down performance wise.

Alternative, we improved the solution by being a bit smarter:

  1. Delete duplicate declarations that have the same property and value (this will get rid of exact duplications).
  2. Delete declarations with the same property and the same unit.

This means that we will reduce this:

.example {
  height: 50%;
  height: 100px;
  height: 20vh;
  height: 30%;
  height: 50px;
  height: 30vh;
  transform: var(--value);
  transform: var(--value);
}

To:

  .example {
-   height: 50%;    /* Another height exists later with a `%` unit */
-   height: 100px;  /* Another height exists later with a `px` unit */
-   height: 20vh;   /* Another height exists later with a `vh` unit */
    height: 30%;
    height: 50px;
    height: 30vh;
-   transform: var(--value); /* Value is too complex, but is **exactly** the same as the one below */
    transform: var(--value);
  }

This will reduce the values that we are 100% sure that can be safely removed. This will still result in some overrides but the browser can handle those for you.

We do pay a small performance price because we now need to analyse each value to know the unit, but this should be neglectable because we will only do this on actual duplicated properties. If you don't have duplicate properties, then this change won't change anything.

Fixes: #6844

In theory, we don't have to do anything because the browser is smart
enough to figure everything out. However, leaving in duplicate
properties is not that ideal for file size.

Our previous method was pretty simple: if you see a declaration you
already saw in this rule, delete the previous one and keep the current
one.

This works pretty well, but this gets rid of **all** the declarations
with the same property. This is not great for overrides for older
browsers.

In a perfect world, we can handle this based on your target browser but
this is a lot of unnecessary complexity and will slow things down
performance wise.

Alternative, we improved the solution by being a bit smarter:
1. Delete duplicate declarations that have the same property and value
   (this will get rid of **exact** duplications).
2. Delete declarations with the same property and the same **unit**.

This means that we will reduce this:
```css
.example {
  height: 50%;
  height: 100px;
  height: 20vh;
  height: 30%;
  height: 50px;
  height: 30vh;
  transform: var(--value);
  transform: var(--value);
}
```

To:
```diff-css
  .example {
-   height: 50%;    /* Another height exists later with a `%` unit */
-   height: 100px;  /* Another height exists later with a `px` unit */
-   height: 20vh;   /* Another height exists later with a `vh` unit */
    height: 30%;
    height: 50px;
    height: 30vh;
-   transform: var(--value); /* Value is too complex, but is **exactly** the same as the one below */
    transform: var(--value);
  }
```

This will reduce the values that we are 100% sure that can be safely
removed. This will still result in some overrides but the browser can
handle those for you.

Fixes: #6844
@RobinMalfait RobinMalfait changed the title improve collapsing of duplicate properties Improve collapsing of duplicate properties Jan 3, 2022
@RobinMalfait RobinMalfait changed the title Improve collapsing of duplicate properties Improve collapsing of duplicate declarations Jan 3, 2022
@RobinMalfait RobinMalfait merged commit b9af5a9 into master Jan 3, 2022
@RobinMalfait RobinMalfait deleted the fix/6844 branch January 3, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants