update CSS byte validation to 75000 (#31)

* update CSS byte validation to 75000 * review feedback, adding more context to logging * add commented reference to the current byte limit
clay · Sep 26, 2022 · 34636fd · 34636fd
1 parent 1a1a76b
commit 34636fd
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 8 deletions.
diff --git a/lib/media.js b/lib/media.js
@@ -59,20 +59,24 @@ function getContentsOfFiles(fileArray, targetDir, filePrefix) {
  * @param  {array} fileArray   An array of files
  * @param  {string} directory  The directory in which `fs` will look for the file
  * @param  {string} filePrefix The directory path before the filename
+ * @param  {string} uri of the page requested
  * @return {string}
  */
-function combineFileContents(fileArray, directory, filePrefix) {
+function combineFileContents(fileArray, directory, filePrefix, uri) {
   if (!fileArray || !fileArray.length) {
     return false;
   }
 
   // If there are files, retrieve contents
   return getContentsOfFiles(fileArray, directory, filePrefix)
     .then(function (fileContents) {
-      const joined = fileContents.join('');
+      const joined = fileContents.join(''),
+        byteLength = util.getByteLength(joined);
 
-      if (util.getByteLength(joined) > 50000) {
-        log('warn', 'Combined CSS contents are beyond the 50000 byte limit specified by AMPHTML.');
+      if (byteLength > 75000) { // see https://github.com/ampproject/amphtml/issues/26466
+        log('warn', 'Combined CSS contents are beyond the 75000 byte limit specified by AMPHTML.', { uri, byteLength });
+      } else if (byteLength > 70000) {
+        log('info', 'Combined CSS contents are above 70000 bytes, near the 75000 byte limit specified by AMPHTML.', { uri, byteLength });
       }
 
       return `<style amp-custom>${joined}</style>`;
@@ -223,7 +227,7 @@ function configure(options, cacheBuster = '') {
  * @return {Function}
  */
 function injectStyles(state) {
-  const { locals } = state;
+  const { locals, _self } = state;
 
   return function (html) {
     var mediaMap = {};
@@ -239,7 +243,7 @@ function injectStyles(state) {
     // allow site to change the media map before applying it
     if (setup.resolveMedia) mediaMap = setup.resolveMedia(mediaMap, locals) || mediaMap;
 
-    mediaMap.styles = combineFileContents(mediaMap.styles, 'public/css', '/css/');
+    mediaMap.styles = combineFileContents(mediaMap.styles, 'public/css', '/css/', _self);
 
     return bluebird.props(mediaMap)
       .then(combinedFiles => {

diff --git a/lib/media.test.js b/lib/media.test.js
@@ -331,14 +331,14 @@ describe(_.startCase(filename), () => {
       test('logs when the combined file contents are too large', () => {
         jest.spyOn(lib, 'getMediaMap').mockReturnValue(_.cloneDeep(cssMediaMap));
         jest.spyOn(util, 'readFilePromise').mockReturnValue(Promise.resolve(styleString));
-        jest.spyOn(util, 'getByteLength').mockReturnValue(50001);
+        jest.spyOn(util, 'getByteLength').mockReturnValue(75001);
 
         return fn(state)(basicHtml)
           .then(() => {
             expect(fakeLog.mock.calls).toEqual([
               [
                 'warn',
-                'Combined CSS contents are beyond the 50000 byte limit specified by AMPHTML.'
+                'Combined CSS contents are beyond the 75000 byte limit specified by AMPHTML.'
               ]
             ]);
           });