Skip to content

Updated the example for mocking the localstorage #6882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

email2vimalraj
Copy link

The mocking example for the localStorage which was provided earlier was breaking after the recent jest update (using CRA - v3.0). I've updated the example to mock through the Storage api.

Sample code sandbox: https://codesandbox.io/s/43pn52xmz0?fontsize=14&previewwindow=tests

The mocking example for the `localStorage` which was provided earlier was breaking after the recent `jest` update (using `CRA - v3.0`). I've updated the example to mock through the `Storage` api.
Sample code sandbox: https://codesandbox.io/s/43pn52xmz0?fontsize=14&previewwindow=tests
@email2vimalraj
Copy link
Author

Look like the travis is hung.

@iansu
Copy link
Contributor

iansu commented Apr 24, 2019

Is this documented somewhere with Jest? I'd like to verify that this is correct before we change it.

@iansu iansu self-assigned this Apr 24, 2019
@email2vimalraj
Copy link
Author

I couldn't find anything related to this in their documentation. However whatever the snippet provided in our documentation doesn't work after the upgrade. The code sandbox link proves that.

@email2vimalraj
Copy link
Author

@iansu : Any comments?

@ianschmitz
Copy link
Contributor

We were on an old version of Jest/jsdom previously which was lacking a localStorage implementation. Now that we're on jsdom@14 it has an implementation for localStorage baked in.

Some relevant info:
jestjs/jest#6798
jsdom/jsdom#2318

@ianschmitz
Copy link
Contributor

This may be a good solution: jestjs/jest#6798 (comment). It isn't overriding the implementation of localStorage so seems a bit cleaner than what's proposed in this PR.

@email2vimalraj
Copy link
Author

Sorry for not replying or not taking any action against the comment. Do you want me to resubmit this PR with your recommendation?

@amyrlam amyrlam removed their request for review August 24, 2020 05:35
@pjg
Copy link

pjg commented Jun 23, 2022

Using jest.spyOn(Storage.prototype, 'setItem'); is a much better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants