Fix memory leak #419 #425

packages/universal-cookie/src/Cookies.ts

@@ -33,16 +33,16 @@ export default class Cookies {
     }
   }
 
-  private _checkChanges(newCookies: { [name: string]: Cookie }) {
+  private _checkChanges(previousCookies: { [name: string]: Cookie }) {
     const names = new Set(
-      Object.keys(newCookies).concat(Object.keys(this.cookies)),
+      Object.keys(previousCookies).concat(Object.keys(this.cookies)),
     );
 
     names.forEach((name) => {
-      if (newCookies[name] !== this.cookies[name]) {
+      if (previousCookies[name] !== this.cookies[name]) {
         this._emitChange({
           name,
-          value: readCookie(newCookies[name]),
+          value: readCookie(this.cookies[name]),
         });
       }
     });
@@ -136,7 +136,7 @@ export default class Cookies {
   public addChangeListener(callback: CookieChangeListener) {
     this.changeListeners.push(callback);
 
-    if (this.changeListeners.length === 1) {
+    if (this.HAS_DOCUMENT_COOKIE && this.changeListeners.length === 1) {
       if (typeof window === 'object' && 'cookieStore' in window) {
         (window.cookieStore as any).addEventListener('change', this.update);
       } else {
@@ -151,7 +151,7 @@ export default class Cookies {
       this.changeListeners.splice(idx, 1);
     }
 
-    if (this.changeListeners.length === 0) {
+    if (this.HAS_DOCUMENT_COOKIE && this.changeListeners.length === 0) {
       if (typeof window === 'object' && 'cookieStore' in window) {
         (window.cookieStore as any).removeEventListener('change', this.update);
       } else {

packages/universal-cookie/src/__tests__/Cookies-test.js

@@ -4,6 +4,7 @@ import { cleanCookies } from '../utils';
 describe('Cookies', () => {
   beforeEach(() => {
     cleanCookies();
+    global.TEST_HAS_DOCUMENT_COOKIE = undefined;
   });
 
   describe('constructor()', () => {
@@ -125,8 +126,8 @@ describe('Cookies', () => {
       cookies.addChangeListener(onChange);
       cookies.set('test', 'meow', { path: '/' });
 
-      expect(onChange).toBeCalledTimes(1);
-      expect(onChange).toBeCalledWith({
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({
         name: 'test',
         value: 'meow',
         options: {
@@ -135,15 +136,48 @@ describe('Cookies', () => {
       });
     });
 
+    it('detect if the cookie was externally changed in the browser', async () => {
+      const cookies = new Cookies();
+
+      const onChange = jest.fn();
+      cookies.addChangeListener(onChange);
+
+      document.cookie = `test="${JSON.stringify({ test: true })}"`;
+
+      await new Promise((r) => setTimeout(r, 500));
+
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({
+        name: 'test',
+        value: {
+          test: true,
+        },
+      });
+    });
+
+    it('ignore if the cookie was externally changed in the server', async () => {
+      global.TEST_HAS_DOCUMENT_COOKIE = false;
+      const cookies = new Cookies();
+
+      const onChange = jest.fn();
+      cookies.addChangeListener(onChange);
+
+      document.cookie = 'test=meow';
+
+      await new Promise((r) => setTimeout(r, 500));
+
+      expect(onChange).not.toHaveBeenCalled();
+    });
+
     it('keeps the value as original object', () => {
       const cookies = new Cookies();
 
       const onChange = jest.fn();
       cookies.addChangeListener(onChange);
       cookies.set('test', [0, 1, 2], { path: '/' });
 
-      expect(onChange).toBeCalledTimes(1);
-      expect(onChange).toBeCalledWith({
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({
         name: 'test',
         value: [0, 1, 2],
         options: {
@@ -159,7 +193,7 @@ describe('Cookies', () => {
       cookies.addChangeListener(onChange);
       cookies.set('test', 'test');
 
-      expect(onChange).toBeCalledWith({
+      expect(onChange).toHaveBeenCalledWith({
         name: 'test',
         value: 'test',
         options: {
@@ -175,7 +209,7 @@ describe('Cookies', () => {
       cookies.addChangeListener(onChange);
       cookies.set('test', 'test', { path: '/woof' });
 
-      expect(onChange).toBeCalledWith({
+      expect(onChange).toHaveBeenCalledWith({
         name: 'test',
         value: 'test',
         options: {
@@ -191,8 +225,8 @@ describe('Cookies', () => {
       cookies.addChangeListener(onChange);
       cookies.remove('test', { path: '/' });
 
-      expect(onChange).toBeCalledTimes(1);
-      expect(onChange).toBeCalledWith({
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({
         name: 'test',
         value: undefined,
         options: {
@@ -211,7 +245,7 @@ describe('Cookies', () => {
       cookies.removeChangeListener(onChange);
 
       cookies.remove('test', 'boom!');
-      expect(onChange).not.toBeCalled();
+      expect(onChange).not.toHaveBeenCalled();
     });
   });
 });

packages/universal-cookie/src/utils.ts

@@ -2,6 +2,11 @@ import * as cookie from 'cookie';
 import { Cookie, CookieGetOptions } from './types';
 
 export function hasDocumentCookie() {
+  const testingValue = (global as any).TEST_HAS_DOCUMENT_COOKIE;
+  if (typeof testingValue === 'boolean') {
+    return testingValue;
+  }
+
   // Can we get/set cookies on document.cookie?
   return typeof document === 'object' && typeof document.cookie === 'string';
 }

setup-tests.ts

@@ -1,5 +1,5 @@
-import { ArrayBuffer, TextDecoder, TextEncoder, Uint8Array } from 'util';
 import '@testing-library/jest-dom';
+import { TextEncoder, TextDecoder } from 'util';
 
 global.TextEncoder = TextEncoder;
 global.TextDecoder = TextDecoder;