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

Global scope object should be window when using happy-dom #1243

Closed
6 tasks done
mikestopcontinues opened this issue May 5, 2022 · 5 comments · Fixed by #1258
Closed
6 tasks done

Global scope object should be window when using happy-dom #1243

mikestopcontinues opened this issue May 5, 2022 · 5 comments · Fixed by #1258

Comments

@mikestopcontinues
Copy link
Contributor

Describe the bug

One of my react hooks relies on a third party library (react-use) to function. That library references the raw global cancelAnimationFrame(), not window.cancelAnimationFrame(). This is perfectly valid JS DOM code, but it throws an error in vitest, because the global scope object doesn't contain cancelAnimationFrame() since it is not window.

I think that the global scope object should be window when using happy-dom (or jsdom, though I'm not using it, so I'm not sure if it's working already).

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-uwd1uk?file=test/basic.test.ts

System Info

System:
    OS: macOS 12.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 161.24 MB / 64.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.19.0 - ~/.nvm/versions/node/v14.19.0/bin/node
    Yarn: 3.2.0 - ~/.nvm/versions/node/v14.19.0/bin/yarn
    npm: 8.5.0 - ~/.nvm/versions/node/v14.19.0/bin/npm
  Browsers:
    Brave Browser: 98.1.35.101
    Chrome: 101.0.4951.54
    Firefox: 99.0
    Safari: 15.4
  npmPackages:
    vitest: 0.10.4 => 0.10.4

Used Package Manager

yarn

Validations

@sheremet-va
Copy link
Member

It is impossible to do with the current architecture. Right now we are copying the Window object properties to global. If they are not copied, it could mean they are not returned from getOwnPropertyNames on window, or not in our jsdom-keys array.

PR welcome to add the key on OTHER_KEYS in jsdom-keys

@mikestopcontinues
Copy link
Contributor Author

@sheremet-va I'm happy to make a PR! Question though: Those two arrays are getting concatenated. What about looping over the enumerable props on happy-dom's (and jsdom's) Window? That way as they fill in API gaps themselves, we automatically get the benefits? As far as I can tell, happy-dom is still playing catch up—and there's always new DOM APIs coming. What do you think?

@sheremet-va
Copy link
Member

@sheremet-va I'm happy to make a PR! Question though: Those two arrays are getting concatenated. What about looping over the enumerable props on happy-dom's (and jsdom's) Window? That way as they fill in API gaps themselves, we automatically get the benefits? As far as I can tell, happy-dom is still playing catch up—and there's always new DOM APIs coming. What do you think?

We are already looping over them tho? See utils.

@mikestopcontinues
Copy link
Contributor Author

mikestopcontinues commented May 6, 2022

Ohh, you are! It looks like the issue is that happy-dom GlobalWindow extends Window, where cancelAnimationFrame is located. My first thought was that Object.getOwnPropertyNames is missing the parent class's props, but in fact the method should be grabbing them just fine.

Is this why you resorted to a manual in the first place? If so, I'll happily just PR for some additional keys. But I'd really like to solve for future cases as well.

@sheremet-va
Copy link
Member

I honestly don’t remember the exact reason. But some properties should not be moved to global space, as they will conflict with happy-dom. (setTimeout will go into infinite loop, for example)

I prefer if we just add a key right now. Also I’ve changed how global is populated in #1256, maybe I will invest more time with globals after we resolve react issues.

mikestopcontinues added a commit to mikestopcontinues/vitest that referenced this issue May 7, 2022
chaii3 pushed a commit to chaii3/vitest that referenced this issue May 13, 2022
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants