capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.14k stars 189 forks source link

[Bug] Unexpected return value when trying to get a value for the key 'key' in LocalStorage before definition #1351

Closed nlicalzi closed 4 months ago

nlicalzi commented 4 months ago

Describe the bug Possibly introduced in: https://github.com/capricorn86/happy-dom/pull/1254/files

We noticed this when running a dependabot bump from version 13.6.2 to 14.3.1.

Calling window.localStorage.getItem('key'); results in unexpected behavior when a KV pair with that key has not yet been set: it returns Function: key instead of the expected string | null return value for the getItem function.

To Reproduce Steps to reproduce the behavior:

Normal behavior:

window.localStorage.setItem('key', 'value');
const item = window.localStorage.getItem('key');
console.log(item); // 'value'

Problematic behavior:

const item = window.localStorage.getItem('key');
console.log(item); // [Function: key]

Expected behavior See above 😄

Device:

capricorn86 commented 4 months ago

Thank you for reporting @nlicalzi! :slightly_smiling_face:

There is a fix in now: https://github.com/capricorn86/happy-dom/releases/tag/v14.3.3

nlicalzi commented 4 months ago

What a fast fix, thank you David!

kolaente commented 3 months ago

Unfortunately, it seems like this broke a test case like this (using vitest):

test('return a saved history', () => {
    const saved = [{id: 1}, {id: 2}]
    Storage.prototype.getItem = vi.fn(() => JSON.stringify(saved))

    const h = getHistory()
    expect(h).toStrictEqual(saved)
})

where getHistory looks like this:

export function getHistory(): ProjectHistory[] {
    const savedHistory = localStorage.getItem('projectHistory')
    if (savedHistory === null) {
        return []
    }

    return JSON.parse(savedHistory)
}

Not sure if I'm doing anything wrong here? It worked fine until happy-dom 14.3.3.

capricorn86 commented 3 months ago

@kolaente Sorry to hear that it broke your test.

window.localStorage and window.sessionStorage are already instantiated and defined during this stage and to be able to work with functionality such as getting a property by key (e.g. window.localStorage['myKey']) the storage had to be changed to a Proxy.

Would changing your spy to this example solve your problem?

vi.spyOn(localStorage, 'setItem').mockImplementation(() => JSON.stringify(saved));
kolaente commented 3 months ago

@capricorn86 That works great. Thank you very much for the fast response!

capricorn86 commented 3 months ago

@kolaente I managed to make spying on the Storage.prototype methods work as well now, if you rather want to go with that solution: https://github.com/capricorn86/happy-dom/releases/tag/v14.6.2