Repository: mapbox/batfish
Pull request #366 (2021-11-19 17:09:13)

Update postcss plugins to skip hashes
This PR fixes a mishap in #364 where the fix was improperly employed to the plugin that serves absolute urls, when it should happen on the plugin that serves relative urls.

In this PR, Batfish will decode the `asset.url` and  then, in addition to absolute paths,  it will skip hashes. This will prevent a relative path from being appended to `mask` paths.

- This commit shows the bug: 5b73dfc
- This commit shows the fix: af24122

This PR will also revert #364


Patch:
diff --git a/src/node/get-postcss-plugins.js b/src/node/get-postcss-plugins.js
index e238773..479cb03 100644
--- a/src/node/get-postcss-plugins.js
+++ b/src/node/get-postcss-plugins.js
@@ -23,8 +23,9 @@ function getPostcssPlugins(
     // inlined CSS (in the static build) and the stylesheet itself.
     postcssUrl({
       url: (asset) => {
-        const parsedUrl = url.parse(asset.url);
-        if (parsedUrl.protocol) {
+        const parsedUrl = url.parse(decodeURIComponent(asset.url));
+        // skip absolute paths and hashes
+        if (parsedUrl.protocol || parsedUrl.hash === parsedUrl.href) {
           return asset.url;
         }
         return joinUrlParts(
diff --git a/src/node/postcss-absolute-urls.js b/src/node/postcss-absolute-urls.js
index 166d1e4..4757f28 100644
--- a/src/node/postcss-absolute-urls.js
+++ b/src/node/postcss-absolute-urls.js
@@ -8,7 +8,7 @@ const postcssUrl = require('postcss-url');
 // PostCSS plugin that transforms all the URLs in some CSS to absolute URLs.
 function postcssAbsoluteUrls(options: { stylesheetUrl: string }): Function {
   const transformUrl = (originalUrl: string): string => {
-    if (/^data:|^%23clip/.test(originalUrl)) return originalUrl;
+    if (/^data:/.test(originalUrl)) return originalUrl;
     return url.resolve(options.stylesheetUrl, originalUrl);
   };
 


Test patch:
diff --git a/test/__snapshots__/compile-stylesheets.test.js.snap b/test/__snapshots__/compile-stylesheets.test.js.snap
index 22f68c3..3400c33 100644
--- a/test/__snapshots__/compile-stylesheets.test.js.snap
+++ b/test/__snapshots__/compile-stylesheets.test.js.snap
@@ -10,8 +10,9 @@ exports[`compileStylesheets minimizes in production 1`] = `
     
 @font-face{font-family:'a';src:url(/mock/base/path/assets/a_994d9585.woff2) format('woff2')}.a{color:pink}
 .b{color:orange}
+a.mapboxgl-ctrl-logo{width:88px;height:23px;margin:0 0 -4px -4px;display:block;background-repeat:no-repeat;cursor:pointer;overflow:hidden;background-image:url(\\"data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23clip)'\\")}
 @font-face{font-family:'c';src:url(/mock/base/path/assets/c_675d172a.woff2) format('woff2')}.c{color:purple}
-@font-face{font-family:'e';src:url(/mock/base/path/assets/e_24153b0b.woff2) format('woff2')}.e{color:gray}/*# sourceMappingURL=batfish-styles-8ca264391785d90b2c9c63b3c7833092.css.map */"
+@font-face{font-family:'e';src:url(/mock/base/path/assets/e_24153b0b.woff2) format('woff2')}.e{color:gray}/*# sourceMappingURL=batfish-styles-832d6b401a65f3d5b345c40f7fc06430.css.map */"
 `;
 
 exports[`compileStylesheets writes expected CSS file 1`] = `
@@ -31,6 +32,17 @@ exports[`compileStylesheets writes expected CSS file 1`] = `
 
 .b { color: orange; }
 
+a.mapboxgl-ctrl-logo {
+  width: 88px;
+  height: 23px;
+  margin: 0 0 -4px -4px;
+  display: block;
+  background-repeat: no-repeat;
+  cursor: pointer;
+  overflow: hidden;
+  background-image: url(\\"data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23clip)'\\");
+}
+
 @font-face {
   font-family: 'c';
   src: url('/mock/base/path/assets/c_675d172a.woff2') format('woff2');
diff --git a/test/__snapshots__/postcss-absolute-urls.test.js.snap b/test/__snapshots__/postcss-absolute-urls.test.js.snap
index bc0cd56..4328912 100644
--- a/test/__snapshots__/postcss-absolute-urls.test.js.snap
+++ b/test/__snapshots__/postcss-absolute-urls.test.js.snap
@@ -1,20 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`postcssAbsoluteUrls do not add absolute url to \`mask\` 1`] = `
-"
-    a.mapboxgl-ctrl-logo {
-      width: 88px;
-      height: 23px;
-      margin: 0 0 -4px -4px;
-      display: block;
-      background-repeat: no-repeat;
-      cursor: pointer;
-      overflow: hidden;
-      background-image: url(\\"data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23clip)'\\");
-    }    
-    "
-`;
-
 exports[`postcssAbsoluteUrls works 1`] = `
 "
       @font-face {
diff --git a/test/fixtures/stylesheets/mapbox-gl.css b/test/fixtures/stylesheets/mapbox-gl.css
new file mode 100644
index 0000000..6fd3162
--- /dev/null
+++ b/test/fixtures/stylesheets/mapbox-gl.css
@@ -0,0 +1,10 @@
+a.mapboxgl-ctrl-logo {
+  width: 88px;
+  height: 23px;
+  margin: 0 0 -4px -4px;
+  display: block;
+  background-repeat: no-repeat;
+  cursor: pointer;
+  overflow: hidden;
+  background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23clip)'");
+}
diff --git a/test/postcss-absolute-urls.test.js b/test/postcss-absolute-urls.test.js
index 09869ff..7e76bef 100644
--- a/test/postcss-absolute-urls.test.js
+++ b/test/postcss-absolute-urls.test.js
@@ -27,30 +27,4 @@ describe('postcssAbsoluteUrls', () => {
         expect(result.css).toMatchSnapshot();
       });
   });
-
-  test('do not add absolute url to `mask`', () => {
-    const css = `
-    a.mapboxgl-ctrl-logo {
-      width: 88px;
-      height: 23px;
-      margin: 0 0 -4px -4px;
-      display: block;
-      background-repeat: no-repeat;
-      cursor: pointer;
-      overflow: hidden;
-      background-image: url("data:image/svg+xml;charset=utf-8,%3Csvg mask='url(%23clip)'");
-    }    
-    `;
-
-    return postcss()
-      .use(
-        postcssAbsoluteUrls({
-          stylesheetUrl: '/playground/assets/'
-        })
-      )
-      .process(css, { from: undefined })
-      .then((result) => {
-        expect(result.css).toMatchSnapshot();
-      });
-  });
 });