Merge pull request #300 from NitroPack/fix/inverse-media-queries

Change how inversed media queries are handled
pocketjoso · May 1, 2020 · 67f66e1 · 67f66e1
2 parents bcfc292 + ea17f93
commit 67f66e1
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 14 deletions.
diff --git a/src/non-matching-media-query-remover.js b/src/non-matching-media-query-remover.js
@@ -24,16 +24,17 @@ function _isMatchingMediaQuery (mediaQuery, matchConfig) {
     // rather than the inverse field, but for our purposes we want to treat
     // them the same.
     const isInverse = mq.inverse || mq.type === 'not'
-    if (
-      (!isInverse && mq.type === 'print') ||
-      (isInverse && mq.type === 'screen')
-    ) {
-      return false
-    }
     // f.e. @media all {}
     // go for false positives over false negatives,
     // i.e. accept @media randomThing {}
     if (mq.expressions.length === 0) {
+      // the checks below are only valid if the media query has no other properties other than the media type
+      if (
+        (!isInverse && mq.type === 'print') ||
+        (isInverse && mq.type === 'screen')
+      ) {
+        return false
+      }
       return true
     }
 
@@ -46,19 +47,27 @@ function _isMatchingMediaQuery (mediaQuery, matchConfig) {
     */
     return mq.expressions.some(function ({ modifier, feature, value }) {
       if (modifier === 'min') {
-        const constructedQuery = `${
-          isInverse ? 'not ' : ''
-        }(min-${feature}: ${value})`
-        return cssMediaQuery.match(constructedQuery, matchConfig)
+        const constructedQuery = `(min-${feature}: ${value})`
+        // css-mediaquery does not match inversed queries correctly, hence the if..else below
+        if (!isInverse) {
+          return cssMediaQuery.match(constructedQuery, matchConfig)
+        } else {
+          return !cssMediaQuery.match(constructedQuery, matchConfig)
+        }
       } else {
         if (mq.expressions.length > 1) {
           const constructedQuery = mq.expressions
             .map(
               ({ modifier, feature, value }) =>
-                `${isInverse ? 'not ' : ''}(${modifier}-${feature}: ${value})`
+                `(${modifier}-${feature}: ${value})`
             )
             .join(' and ')
-          return cssMediaQuery.match(constructedQuery, matchConfig)
+          // css-mediaquery does not match inversed queries correctly, hence the if..else below
+          if (!isInverse) {
+            return cssMediaQuery.match(constructedQuery, matchConfig)
+          } else {
+            return !cssMediaQuery.match(constructedQuery, matchConfig)
+          }
         } else {
           return true
         }

diff --git a/test/pre-formatting.test.js b/test/pre-formatting.test.js
@@ -33,13 +33,17 @@ describe('penthouse pre formatting tests', () => {
       `@media oiasjdoiasd {}`,
       // covering combined queries
       `@media (min-width: 320px) and (max-width: 400px) {}`,
-      `@media (min-width: 150px) and (max-width: 1700px) {}`
+      `@media (min-width: 150px) and (max-width: 1700px) {}`,
+      `@media not screen and (min-width: 1800px) {}`,
+      `@media not all and (min-width: 101em) {}`
     ]
     // 1300, 1600
     const mediaToRemoveAlways = [
       `@media print {}`,
       `@media not screen {}`,
-      `@media not (min-width: 1px) {}`
+      `@media not (min-width: 1px) {}`,
+      `@media not screen and (min-width: 800px) {}`,
+      `@media not all and (min-width: 50em) {}`
     ]
     // 1600
     const mediaToRemoveUnlessLarge = [