jsdom update 5 days ago breaks back compatability

[vlad0]

🐛 Bug Report

jest.spyOn(localStorage, "setItem"); doesn't work;

jest-environment-jsdom has caret dependency on jsdom -> 11.5.1, jsdom latest version is 11.12.0 and it includes localStorage and sessionStorage. Previously we could mock localStorage easily but now we don't have that option and we need to use the default localStorage implementation. So our option is to spy on it which doesn't work because it throws TypeError: object[methodName].mockImplementation is not a function

What I have discovered is that in ./node_modules/jsdom/lib/jsdom/living/generated/Storage.js the issue is with the set method of the Proxy

Currently the code below is on line 278.

if (ownDesc === undefined) {
          const parent = Reflect.getPrototypeOf(target);
          if (parent !== null) {
            return Reflect.set(parent, P, V, receiver);
          }
          ownDesc = { writable: true, enumerable: true, configurable: true, value: undefined };
        }

if we remove receiver from return Reflect.set(parent, P, V, receiver); we will be able to spy on it. But I guess that's coming from webidl converter

To Reproduce

Steps to reproduce the behavior:

1. Install jest.
2. Write a simple class that leverage localStorage.setItem() or localStorage.getItem()
3. Try to spy on it -> jest.spyOn(localStorage, "setItem"); and it will throw an error

Expected behavior

The method should be available for spying.

Link to repl or repo (highly encouraged)

https://github.com/vlad0/jest-localstorage-issue

Run npx envinfo --preset jest

1. npm install
2. npm test
   Paste the results here:

> jest

 FAIL  ./service.spec.js
  Service
    ✕ Service set value (7ms)

  ● Service › Service set value

    TypeError: object[methodName].mockImplementation is not a function

       8 |
       9 |     it('Service set value', () => {
    > 10 |         jest.spyOn(localStorage, "setItem");
      11 |         service.setValue("hello", "world")
      12 |
      13 |         expect(localStorage.setItem).toHaveBeenCalled();

      at ModuleMockerClass.spyOn (node_modules/jest-mock/build/index.js:597:26)
      at Object.it (service.spec.js:10:14)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.785s, estimated 1s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

[SimenB]

Not much for jest to do here, I'm afraid... You can always rollback jsdom and use a lockfile

[vlad0]

@SimenB , to be honest I was really confused where to raise this issue but since jest-environment-jsdom is part of this repo and it has caret dependency to jsdom which causes the issue I decided to give it a try here.

Let me give another perspective:

Imagine I am a new Jest user and want to spy on localStorage - What are our options to spy on localStorage with the latest version of Jest? - after all it downloads the latest version of jsdom which can't be spied on or mocked.

There are lots of example with mocking localStorage before jsdom implementation but no solution after the lastest jsdom update.

UPDATE: jsdom team suggests the fix should come from jest :) so we seem to be in the limbo :)
jsdom/jsdom#2318

[mr-wildcard]

@vlad0 did you manage to mock/spy anything from jsdom's localStorage implementation ?

Since jsdom released its 11.12.0, if I recall, there is no workaround for this :/ kind of a blocker when you want to unit test...

[SimenB]

Like they said in the jsdom issue, how would this be mocked in the browser? If it's impossible to mock in the browser we might be at an impasse. There is nothing we can do inside Jest that you cannot do in userland, but if you're able to mock it using plain JSDOM then we can look into porting the solution into Jest

[mr-wildcard]

I understand the pov of @domenic on the jsdom side, and I agree this should not be fixed by them.

Maybe I'm missing something, but how comes that I can jest.spyOn on JSON.parse for example, and not on localStorage's methods ?

I'm getting the same error than @vlad0 :

TypeError: object[methodName].mockImplementation is not a function

Edit : with Jasmine's global spyOn it's working..

[SimenB]

I added a comment over there. Jsdom has different behaviour from chrome - the function is not possible to replace by simple assignment, which is what Jest does (or tries to do) see https://github.com/facebook/jest/blob/bed7e17ff0900320ef8409f5453836a94bb8456d/packages/jest-mock/src/index.js#L762-L768. Line 766 is the one that throws since the assignment on line 762 silently fails

[SimenB]

Ok, that's apparently a bug in Chrome. If anyone is able to somehow replace a function on localStorage in Safari (or Edge), then we can look into porting that over to Jest so it can handle it automatically.

I'll send a PR throwing an error when trying to spy on something that's not replacable

[mr-wildcard]

We found out that localStorage from Jest is an instance of a class called Storage provided by jsdom.
And using spyOn this way works :

jest.spyOn(Storage.prototype, 'setItem');

[SimenB]

Spying on the prototype is an interesting solution.

@thymikee @rickhanlonii thoughts on falling back to trying to spy on the prototype? My local diff is this:

diff --git i/packages/jest-mock/src/index.js w/packages/jest-mock/src/index.js
index 9b679c73e..cedbe376a 100644
--- i/packages/jest-mock/src/index.js
+++ w/packages/jest-mock/src/index.js
@@ -763,6 +763,10 @@ class ModuleMockerClass {
         object[methodName] = original;
       });
 
+      if (object[methodName] === original) {
+        throw new Error(`Unable to mock method \`${methodName}\``);
+      }
+
       object[methodName].mockImplementation(function() {
         return original.apply(this, arguments);
       });

and a test, but in theory we could try some more fancy stuff by looking up constructors etc

[thymikee]

What if your obj/fn overrides the prototype? You'll end up mocking different functions, no?

[moimael]

@mr-wildcard Thanks a lot for the workaround ! How would you distinguish between local and sessionStorage though ? I'd like to check that my data is saved in the right storage area in my tests.

[tamlyn]

Something to bear in mind is now that jsdom provides a working localStorage implementation, you may no longer need to mock it. I just update my tests to assert on the value retrieved via getItem instead.

[mr-wildcard]

@moimael good question, I didn't think about sessionStorage. I can't give it a try right now but if both sessionStorage and localStorage are instantiated from a generic Storage class, then it could be problematic to distinguish them... 🤔

@tamlyn yes, but in this case you're not unit testing your code anymore because localStorage is not mocked and by doing that way you now need to take care of clearing mutated state with afterEach beforeEach & co. Which is not really the kind of isolation in which I want to test some of my business code.

[tomprats]

Here's a solution I adapted from a similar problem mocking window.location #5124

describe("sessionStorage", ()=>{
  let originalSessionStorage;

  beforeAll(()=>{
    originalSessionStorage = window.sessionStorage;
    delete window.sessionStorage;
    Object.defineProperty(window, "sessionStorage", {
      writable: true,
      value: {
        getItem: jest.fn().mockName("getItem"),
        setItem: jest.fn().mockName("setItem")
      }
    });
  });

  beforeEach(()=>{
    sessionStorage.getItem.mockClear();
    sessionStorage.setItem.mockClear();
  });

  afterAll(()=>{
    Object.defineProperty(window, "sessionStorage", {writable: true, value: originalSessionStorage});
  });

  it("calls getItem", ()=>{
    sessionStorage.getItem("abc");

    expect(sessionStorage.getItem).toHaveBeenCalledWith("abc");
  });
});

[omalraj]

jest.spyOn(window.localStorage.__proto__, 'setItem'); should work without any additional imports.

[mr-wildcard]

`read` method doesn't exist on LocalStorage prototype. You're probably looking for `getItem` instead.

[corey-aloia]

@mr-wildcard I am so embarrassed that I had to delete my comment. It actually looks like a big plus that we can't overwrite this, otherwise someone might accidentally create a mock with read instead of getItem :).
jest.spyOn(Object.getPrototypeOf(window.localStorage), 'getItem');
Works great, and is, IMO, an improvement over the older version of Jest/JSDom where we had to do this manually :)

[j18s]

How about this, what do you guys think about below code pluses and deltas:

const localStorage = jest.fn(); localStorage.getItem = () => 'abc....'; console.log(localStorage.getItem());

similarly we can set the item .... I haven't tried setItem yet but will try ...

[SimenB]

If people can come up with some way we can handle this generically in jest-mock, I'm happy to reopen. As is, I don't think this is actionable, so I'm closing this. Please keep discussing, though!

[maneeshpal]

technically, Storage.prototype.setItem is exactly the same as localStorage.__proto__.setItem.
So, mocking localStorage.__prop__.setItem doesn't mean you are targeting localStorage.
This mock will also apply for any sessionStorage.setItem calls within your unit of test.

[mayank23]

Here's a utility function for mocking any window property with automatic cleanup:

https://gist.github.com/mayank23/7b994385eb030f1efb7075c4f1f6ac4c

[baipi]

Thanks @mayank23, you made my day!

An example for those who would like to return a specific value according to the test case:

// file.js
const funcToTest = () => {
  const { localStorage } = window;
  return JSON.parse(localStorage.getItem('toto'));
};

export { funcToTest };

// file.test.js
import mockWindowProperty from './mock-window-property';
import { funcToTest } from './file';

describe('funcToTest', () => {
  let mockGetItem;

  mockWindowProperty('localStorage', {
    getItem: params => mockGetItem(params),
  });

  beforeEach(() => {
    mockGetItem = jest.fn().mockReturnValue(null);
  });

  it('Should do something with id', (done) => {
    mockGetItem = jest.fn().mockReturnValue('{"id": "1"}');
    const res = funcToTest();
    expect(mockGetItem.mock.calls.length).toBe(1);
    expect(mockGetItem.mock.calls[0][0]).toBe('toto');
    expect(res).toStrictEqual({ id: 1 });
    // ...
  });
});

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.