[normalize-whitespace]: removal of whitespace in at-rule and params

[anikethsaha]

ref #428

Some of the transforms

@media and (...) => @media and(...)
@media screen and () => @media screen and()
@media ( max-width:   50px ) => @media (max-width:50px)

[codecov-commenter]

Codecov Report

Merging #921 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #921      +/-   ##
==========================================
+ Coverage   97.26%   97.27%   +0.01%     
==========================================
  Files         119      119              
  Lines        3477     3492      +15     
  Branches     1047     1050       +3     
==========================================
+ Hits         3382     3397      +15     
  Misses         87       87              
  Partials        8        8              

Impacted Files	Coverage Δ
packages/postcss-normalize-whitespace/src/index.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43f3423...5637f95. Read the comment docs.

[alexander-akait]

@media () => @media() -> need test on IE
@media and () => @media and() -> invalid
@media ( max-width: 50px ) => @media (max-width:50px) -> good idea, but we should uses parser or very very very strict regex + many tests
@support () , @support () => @support(),@support() -> i think it is invalid syntax, no examples in spec, but maybe I wrong

[anikethsaha]

@media () => @media() -> need test on IE

ok

@media and () => @media and() -> invalid

changed the PR description,

@support () , @support () => @support(),@support()

yes invalid, I removed it from the PR description, I couldn't find any example in the spec, but postcss should throw parsing error right if this is an invalid syntax ?

@media ( max-width: 50px ) => @media (max-width:50px) -> good idea, but we should uses parser or very very very strict regex + many tests

I think the current regex will only convert multiple spaces to a single one. I think it is safe, it won't do any changes to the newlines, neither to tabs

[alexander-akait]

yes invalid, I removed it from the PR description, I couldn't find any example in the spec, but postcss should throw parsing error right if this is an invalid syntax ?

If it is invalid syntax - yes, but I am not usre we need read spec

[alexander-akait]

I think the current regex will only convert multiple spaces to a single one. I think it is safe, it won't do any changes to the newlines, neither to tabs

I am afraid about var() function, it can be part of @supports

[alexander-akait]

We can use postcss-value-parser for this, because syntax is same

[anikethsaha]

If it is invalid syntax - yes, but I am not usre we need read spec

the spec says,

@supports <supports-condition> {
  <stylesheet>
}

so I suppose there can be nested at-rules, but no atules in <supports-condition>

I am afraid about var() function, it can be part of @supports

var does supports spaces inside of the (..) so even removing them should not be unsafe. I will add some test to show that.

We can use postcss-value-parser for this, because syntax is same

does it supports parsing at rules ?

yea it does, though I will check it if it is better to use this or the regex.

[anikethsaha]

@evilebottnawi I think there is some issue if we go with AST.
In AST, it parse the spaces as a node, so for example

@media screen and ( max-width: 500px) {
}

this is the tree

    ValueParser {
      nodes: [
        { type: 'word', sourceIndex: 0, value: 'screen' },
        { type: 'space', sourceIndex: 6, value: ' ' },
        { type: 'word', sourceIndex: 7, value: 'and' },
        { type: 'space', sourceIndex: 10, value: ' ' },
        {
          type: 'function',
          sourceIndex: 11,
          value: '',
          before: ' ',
          after: '',
          nodes: [
            { type: 'word', sourceIndex: 13, value: 'max-width' },
            { type: 'div', sourceIndex: 22, value: ':', before: '', after: '' },
            { type: 'word', sourceIndex: 23, value: '500px' }
         ]
        }
      ]
    }

it would be unsafe to remove all the space.

we can remove the whitespace inside of the function i.e inside of the parens (...) but not it would be kind of hacky for and (
we need to check spaces before ( so while traversing the AST, if we encounter ( we can't go back to the previous node.

[alexander-akait]

Can you test this compression on IE8/IE10/IE11, need to investigate

[alexander-akait]

I am afraid @media () => @media() is not valid

[anikethsaha]

Can you test this compression on IE8/IE10/IE11, need to investigate

I have added test for ie11. should we still support ie10, 8?

[anikethsaha]

I am afraid @media () => @media() is not valid

opz, might have forgot to add and I changed it 👍

[alexander-akait]

I mean we should test it manually #921 (comment) 😄

[anikethsaha]

I dont have ie in my machine.

I think we can add a check using browserlist + caniuse-lite. It would be a breaking change then.

[alexander-akait]

@anikethsaha I think it is time to learn virtual box 😄 I'm not sure if this does not work, maybe everything is fine, just need test

[anikethsaha]

I have some bad experience with Virtual box earlier, I do have it installed and my config for that is messed up.

I will try to look for some online VM to test them.