fix: ensure referential integrity, match chromes behavior

calebdwilliams · Dec 28, 2020 · 99bdde3 · 99bdde3
1 parent 393ccdf
commit 99bdde3
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 57 deletions.
diff --git a/src/ConstructStyleSheet.js b/src/ConstructStyleSheet.js
@@ -3,8 +3,9 @@ import {
   frame,
   sheetMetadataRegistry,
   state,
+  OldCSSStyleSheet
 } from './shared';
-import {instanceOfStyleSheet, rejectImports} from './utils';
+import {rejectImports} from './utils';
 
 const cssStyleSheetMethods = [
   'addImport',
@@ -21,37 +22,16 @@ const cssStyleSheetNewMethods = ['replace', 'replaceSync'];
 export function updatePrototype(proto) {
   cssStyleSheetNewMethods.forEach(methodKey => {
     proto[methodKey] = function () {
-      return ConstructStyleSheet.prototype[methodKey].apply(this, arguments);
+      /* This matches Chrome's behavior. Try running this:
+           var style = document.createElement('style');
+           document.head.appendChild(style);
+           style.sheet.replace('body { color: blue }');
+      */
+     return Promise.reject(
+       new Error(`Failed to execute '${methodKey}' on 'CSSStyleSheet': Can't call ${methodKey} on non-constructed CSSStyleSheets.`)
+     );
     }
   });
-
-  // ForEach it because we need to preserve "methodKey" in the created function
-  cssStyleSheetMethods.forEach(methodKey => {
-    // Here we apply all changes we have done to the original CSSStyleSheet
-    // object to all adopted style element.
-    const oldMethod = proto[methodKey];
-
-    proto[methodKey] = function() {
-      const args = arguments;
-      const result = oldMethod.apply(this, args);
-
-      if (sheetMetadataRegistry.has(this)) {
-        const {adopters, actions} = sheetMetadataRegistry.get(this);
-
-        adopters.forEach(styleElement => {
-          if (styleElement.sheet) {
-            styleElement.sheet[methodKey].apply(styleElement.sheet, args);
-          }
-        });
-
-        // And we also need to remember all these changes to apply them to
-        // each newly adopted style element.
-        actions.push([methodKey, args]);
-      }
-
-      return result;
-    };
-  });
 }
 
 function updateAdopters(sheet) {
@@ -63,10 +43,8 @@ function updateAdopters(sheet) {
 }
 
 // This class will be a substitute for the CSSStyleSheet class that
-// cannot be instantiated. The `new` operation will return the native
-// CSSStyleSheet object extracted from a style element appended to the
-// iframe.
-export default class ConstructStyleSheet {
+// cannot be instantiated.
+class ConstructStyleSheet {
   constructor() {
     // A style element to extract the native CSSStyleSheet object.
     const basicStyleElement = document.createElement('style');
@@ -81,26 +59,33 @@ export default class ConstructStyleSheet {
       deferredStyleSheets.push(basicStyleElement);
     }
 
-    const nativeStyleSheet = basicStyleElement.sheet;
-
     // A support object to preserve all the polyfill data
-    sheetMetadataRegistry.set(nativeStyleSheet, {
+    sheetMetadataRegistry.set(this, {
       adopters: new Map(),
       actions: [],
       basicStyleElement,
     });
+  }
 
-    return nativeStyleSheet;
+  get cssRules() {
+    if (!sheetMetadataRegistry.has(this)) {
+      throw new Error(
+        "Cannot read 'cssRules' on non-constructed CSSStyleSheets.",
+      )
+    }
+
+    const {basicStyleElement} = sheetMetadataRegistry.get(this);
+    return basicStyleElement.sheet.cssRules;
   }
 
   replace(contents) {
     const sanitized = rejectImports(contents);
     return new Promise((resolve, reject) => {
       if (sheetMetadataRegistry.has(this)) {
         const {basicStyleElement} = sheetMetadataRegistry.get(this);
-
+        
         basicStyleElement.innerHTML = sanitized;
-        resolve(basicStyleElement.sheet);
+        resolve(this);
         updateAdopters(this);
       } else {
         reject(
@@ -121,7 +106,7 @@ export default class ConstructStyleSheet {
       basicStyleElement.innerHTML = sanitized;
       updateAdopters(this);
 
-      return basicStyleElement.sheet;
+      return this;
     } else {
       throw new Error(
         "Failed to execute 'replaceSync' on 'CSSStyleSheet': Can't call replaceSync on non-constructed CSSStyleSheets.",
@@ -130,7 +115,43 @@ export default class ConstructStyleSheet {
   }
 }
 
+// Implement all methods from the base CSSStyleSheet constructor as
+// a proxy to the raw style element created during construction.
+cssStyleSheetMethods.forEach(method => {
+  ConstructStyleSheet.prototype[method] = function() {
+    if (!sheetMetadataRegistry.has(this)) {
+      throw new Error(
+        `Failed to execute '${method}' on 'CSSStyleSheet': Can't call ${method} on non-constructed CSSStyleSheets.`,
+      )
+    }
+
+    const args = arguments;
+    const { adopters, actions, basicStyleElement } = sheetMetadataRegistry.get(this);
+    const result = basicStyleElement.sheet[method].apply(basicStyleElement.sheet, args);
+
+    adopters.forEach(styleElement => {
+      if (styleElement.sheet) {
+        styleElement.sheet[method].apply(styleElement.sheet, args);
+      }
+    });
+
+    actions.push([method, args]);
+
+    return result;
+  }
+});
+
+export function instanceOfStyleSheet(instance) {
+  return (
+    instance.constructor === ConstructStyleSheet ||
+    instance instanceof OldCSSStyleSheet ||
+    (frame.CSSStyleSheet && instance instanceof frame.CSSStyleSheet)
+  );
+}
+
 Object.defineProperty(ConstructStyleSheet, Symbol.hasInstance, {
   configurable: true,
   value: instanceOfStyleSheet,
 });
+
+export default ConstructStyleSheet;
diff --git a/src/utils.js b/src/utils.js
@@ -1,14 +1,8 @@
-import {adoptedSheetsRegistry, frame, OldCSSStyleSheet} from './shared';
+import {adoptedSheetsRegistry} from './shared';
+import {instanceOfStyleSheet} from './ConstructStyleSheet';
 
 const importPattern = /@import\surl(.*?);/gi;
 
-export function instanceOfStyleSheet(instance) {
-  return (
-    instance instanceof OldCSSStyleSheet ||
-    instance instanceof frame.CSSStyleSheet
-  );
-}
-
 export function checkAndPrepare(sheets, container) {
   const locationType = container === document ? 'Document' : 'ShadowRoot';
 

diff --git a/test/polyfill.test.js b/test/polyfill.test.js
@@ -58,9 +58,8 @@ describe('Constructible Style Sheets polyfill', () => {
 
         const resolved = await result;
 
-        // Equal because polyfill cannot return the same CSSStyleSheet object
-        // since it is immutable.
-        expect(resolved).toEqual(sheet);
+        // Use toBe because there should be referential integrity
+        expect(resolved).toBe(sheet);
       });
 
       it('has a rule set', async () => {
@@ -91,9 +90,8 @@ describe('Constructible Style Sheets polyfill', () => {
       });
 
       it('returns a CSSStyleSheet object itself', () => {
-        // Equal because polyfill cannot return the same CSSStyleSheet object
-        // since it is immutable.
-        expect(result).toEqual(sheet);
+        // Use toBe because there should be referential integrity
+        expect(result).toBe(sheet);
       });
 
       it('has a rule set', async () => {

diff --git a/test/test.js b/test/test.js
@@ -10,10 +10,10 @@ headingStyles.replace(` h1 {
 });
 
 paragraphStyles.replaceSync(`p {
-  color: #1121212;
+  color: #ab2121;
   font-family: "Operator Mono", "Helvetica Neue";
 }`);
 
 setTimeout(() => {
-  headingStyles.addRule('*', 'font-family: Helvetica');
+  headingStyles.addRule('*', 'font-family: monospace');
 }, 1000);