fix: style refcounting bugs and render-phase leak under React 18+/19

README.md

@@ -25,6 +25,23 @@ chat room on Discord to stay up to date
 $ npm install isomorphic-style-loader --save-dev
 ```
 
+## Requirements
+
+- Webpack 5
+- React 16.8 or newer
+- `css-loader` configured with `esModule: false` (the loader reads CSS module
+  class names from `exports.locals`)
+
+The optional `getCss` loader option is serialized into the generated module,
+so it must be a self-contained function without closed-over variables.
+
+## Breaking Changes in v6
+
+- Static properties of the wrapped component (e.g. `fetchData`) are no longer
+  hoisted onto the component returned by `withStyles`. Access them via the
+  `ComposedComponent` property instead.
+- `prop-types` validation has been removed.
+
 ## Getting Started
 
 **Webpack configuration:**
@@ -41,7 +58,8 @@ module.exports = {
           {
             loader: 'css-loader',
             options: {
-              importLoaders: 1
+              importLoaders: 1,
+              esModule: false
             }
           },
           'postcss-loader'

src/createUniqueIdentifiers.js

@@ -12,7 +12,7 @@
  * to duplicates, e.g. ['1', '2', '2'] => ['1', '2', '2_1'].
  */
 function createUniqueIdentifiers(identifiers) {
-  const dupeCount = {}
+  const dupeCount = Object.create(null)
 
   return identifiers.map((identifier) => {
     if (typeof dupeCount[identifier] !== 'undefined') {

src/insertCss.js

@@ -9,14 +9,14 @@
 
 import createUniqueIdentifiers from './createUniqueIdentifiers'
 
-const inserted = {}
+const inserted = Object.create(null)
 
 // Base64 encoding and decoding - The "Unicode Problem"
 // https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#The_Unicode_Problem
 function b64EncodeUnicode(str) {
   return btoa(
     encodeURIComponent(str).replace(/%([0-9A-F]{2})/g, (match, p1) =>
-      String.fromCharCode(`0x${p1}`),
+      String.fromCharCode(parseInt(p1, 16)),
     ),
   )
 }
@@ -28,6 +28,7 @@ function b64EncodeUnicode(str) {
 function removeCss(ids) {
   ids.forEach((id) => {
     if (--inserted[id] <= 0) {
+      delete inserted[id]
       const elem = document.getElementById(id)
       if (elem) {
         elem.parentNode.removeChild(elem)
@@ -51,17 +52,17 @@ function insertCss(styles, { replace = false, prepend = false, prefix = 's' } =
     const [, css, media, sourceMap] = styles[i]
     const id = `${prefix}${identifiers[i]}`
 
-    ids.push(id)
-
     if (inserted[id]) {
       if (!replace) {
         inserted[id]++
+        ids.push(id)
         continue
       }
+    } else {
+      inserted[id] = 1
+      ids.push(id)
     }
 
-    inserted[id] = 1
-
     let elem = document.getElementById(id)
     let create = false
 
@@ -77,19 +78,14 @@ function insertCss(styles, { replace = false, prepend = false, prefix = 's' } =
     }
 
     let cssText = css
-    if (sourceMap && typeof btoa === 'function') {
-      // skip IE9 and below, see http://caniuse.com/atob-btoa
+    if (sourceMap) {
       cssText += `\n/*# sourceMappingURL=data:application/json;base64,${b64EncodeUnicode(
         JSON.stringify(sourceMap),
       )}*/`
       cssText += `\n/*# sourceURL=${sourceMap.file}?${id}*/`
     }
 
-    if ('textContent' in elem) {
-      elem.textContent = cssText
-    } else {
-      elem.styleSheet.cssText = cssText
-    }
+    elem.textContent = cssText
 
     if (create) {
       if (prepend) {
@@ -100,7 +96,12 @@ function insertCss(styles, { replace = false, prepend = false, prefix = 's' } =
     }
   }
 
-  return removeCss.bind(null, ids)
+  let removed = false
+  return () => {
+    if (removed) return
+    removed = true
+    removeCss(ids)
+  }
 }
 
 export default insertCss

src/useStyles.js

@@ -23,7 +23,7 @@ function useStyles(...styles) {
   }
   if (isBrowser) {
     // eslint-disable-next-line react-hooks/rules-of-hooks
-    useLayoutEffect(runEffect, [])
+    useLayoutEffect(runEffect, [insertCss])
   } else {
     runEffect()
   }

src/withStyles.js

@@ -11,12 +11,20 @@ import React from 'react'
 
 import StyleContext from './StyleContext'
 
+const isBrowser = typeof window !== 'undefined'
+
 function withStyles(...styles) {
   return function wrapWithStyles(ComposedComponent) {
     class WithStyles extends React.PureComponent {
       constructor(props, context) {
         super(props, context)
-        this.removeCss = context.insertCss(...styles)
+        if (!isBrowser) {
+          context.insertCss(...styles)
+        }
+      }
+
+      componentDidMount() {
+        this.removeCss = this.context.insertCss(...styles)
       }
 
       componentWillUnmount() {

test/insertCss.test.js

@@ -19,7 +19,7 @@ describe('insertCss(styles, options)', () => {
     expect(removeCss).toBeDefined()
     removeCss()
     style = global.document.getElementById('s1')
-    expect(style).toBeNull
+    expect(style).toBeNull()
   })
 
   it('Should insert and remove multiple <style> elements for a single module', () => {
@@ -52,4 +52,38 @@ describe('insertCss(styles, options)', () => {
     removeCss2()
     expect(global.document.getElementsByTagName('style')).toHaveLength(0)
   })
+
+  it('Should make the returned remove function idempotent', () => {
+    const css = 'body { color: red; }'
+    const removeCss1 = insertCss([[10, css]])
+    const removeCss2 = insertCss([[10, css]])
+    removeCss1()
+    removeCss1()
+    expect(global.document.getElementById('s10')).not.toBeNull()
+    removeCss2()
+    expect(global.document.getElementById('s10')).toBeNull()
+  })
+
+  it('Should not lose subsequent inserts after the count reaches zero', () => {
+    const css = 'body { color: red; }'
+    insertCss([[11, css]])()
+    expect(global.document.getElementById('s11')).toBeNull()
+    const removeCss = insertCss([[11, css]])
+    expect(global.document.getElementById('s11')).not.toBeNull()
+    removeCss()
+    expect(global.document.getElementById('s11')).toBeNull()
+  })
+
+  it('Should preserve the reference count when replacing styles', () => {
+    const removeCss1 = insertCss([[12, 'body { color: red; }']])
+    const removeCss2 = insertCss([[12, 'body { color: red; }']])
+    const removeReplaced = insertCss([[12, 'body { color: blue; }']], { replace: true })
+    expect(global.document.getElementById('s12').textContent).toBe('body { color: blue; }')
+    removeReplaced()
+    expect(global.document.getElementById('s12')).not.toBeNull()
+    removeCss1()
+    expect(global.document.getElementById('s12')).not.toBeNull()
+    removeCss2()
+    expect(global.document.getElementById('s12')).toBeNull()
+  })
 })

test/withStyles.test.js

@@ -40,6 +40,53 @@ describe('withStyles(...styles)(WrappedComponent)', () => {
     expect(insertCss).toHaveBeenCalledTimes(1)
   })
 
+  it('Should pair every insertCss call with exactly one removeCss call in StrictMode', () => {
+    jest.useFakeTimers()
+    try {
+      class Foo extends Component {
+        render() {
+          return <div />
+        }
+      }
+
+      const FooWithStyles = withStyles('')(Foo)
+      const removers = []
+      const insertCss = jest.fn(() => {
+        const removeCss = jest.fn()
+        removers.push(removeCss)
+        return removeCss
+      })
+      const container = global.document.createElement('div')
+
+      const root = createRoot(container)
+      act(() => {
+        root.render(
+          <React.StrictMode>
+            <StyleContext.Provider value={{ insertCss }}>
+              <FooWithStyles />
+            </StyleContext.Provider>
+          </React.StrictMode>,
+        )
+      })
+      act(() => {
+        jest.runAllTimers()
+      })
+      act(() => {
+        root.unmount()
+      })
+      act(() => {
+        jest.runAllTimers()
+      })
+
+      expect(insertCss).toHaveBeenCalled()
+      removers.forEach((removeCss) => {
+        expect(removeCss).toHaveBeenCalledTimes(1)
+      })
+    } finally {
+      jest.useRealTimers()
+    }
+  })
+
   it('Should set the displayName correctly', () => {
     expect(
       withStyles('')(

tools/build.js

@@ -68,10 +68,8 @@ async function build() {
               [
                 '@babel/preset-env',
                 {
-                  corejs: 3,
                   modules: false,
                   loose: true,
-                  useBuiltIns: 'entry',
                 },
               ],
             ],