Skip to content
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

localStorage expected numbers returning strings in [email protected] update #6795

Closed
anselmbradford opened this issue Aug 1, 2018 · 8 comments

Comments

@anselmbradford
Copy link

anselmbradford commented Aug 1, 2018

💥 Regression Report

I work on a project that simulates an object store with this module. The global localStorage is set to this simulated version here. The test checks for a number being set in the store. In 23.4.1 this works. In 23.4.2 I get the error Expected number but received string., and the returned value is "1" and not 1.

Possibly related to the update to resolve jsdom/jsdom#2304

Last working version

Worked up to version: 23.4.1

Stopped working in version: 23.4.2

To Reproduce

Hmm… well you could try https://github.com/cfpb/cfgov-refresh#quickstart, update jest in package.json, and run gulp test:unit

Expected behavior

Data types should not change.

Link to repl or repo (highly encouraged)

See Regression Report.

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS Sierra 10.12.6
    CPU: x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
  Binaries:
    Node: 8.11.1 - ~/.nvm/versions/node/v8.11.1/bin/node
    Yarn: 1.7.0 - ~/homebrew/bin/yarn
    npm: 5.6.0 - ~/.nvm/versions/node/v8.11.1/bin/npm
  npmPackages:
    jest: 23.4.2 => 23.4.2
@thymikee
Copy link
Collaborator

thymikee commented Aug 2, 2018

This is probably a fix in your storageMock library or JSDOM, possibly you'll need to set "testURL": "http://localhost" (see #6792). localStorage should serialize every value to string. It's a pitty it actually accepts non-strings as arguments, because it's misleading.

screen shot 2018-08-02 at 08 14 46

screen shot 2018-08-02 at 08 14 51

@thymikee thymikee closed this as completed Aug 2, 2018
@emceearjun
Copy link

Can anyone tell me if this fix has been published to npm?

@SimenB
Copy link
Member

SimenB commented Aug 2, 2018

There is no bug (or fix) for jest here. Jest has nothing to do with local storage, you probably upgraded jsdom

@anselmbradford
Copy link
Author

There is no bug (or fix) for jest here. Jest has nothing to do with local storage, you probably upgraded jsdom

Looking at the package-lock.json, this is true, jsdom went from 11.11.0 to 11.12.0, I guess because of the caret here. I'll snoop around jsdom for what changed in that minor update. Thanks!

@anselmbradford
Copy link
Author

@SimenB
Copy link
Member

SimenB commented Aug 2, 2018

jsdom implemented localstorage. Your old implementation was buggy, as can be seen by @thymikee's examples

@anselmbradford
Copy link
Author

Yup that was it, thanks for the help!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants