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

Fix jsdom 2 #248

Merged
merged 2 commits into from
Aug 4, 2019
Merged

Fix jsdom 2 #248

merged 2 commits into from
Aug 4, 2019

Conversation

cdow
Copy link

@cdow cdow commented Aug 3, 2019

Purpose (TL;DR) - mandatory

Fix #243 by changing the way the performance field gets overwritten. This time without breaking some browsers.

Background (Problem in detail) - optional

JSDOM has a read only performance field. So, when JSDOM is configured to provide global values, lolex will error on init. This is because it can't overwrite the global performance field.

Copy link
Contributor

@fatso83 fatso83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems great! I'll run the CI tests, just in case, this time 😉

describe("globally configured browser objects", function () {
var withGlobal, originalDescriptors;

// Used to support Node 6 which doesn't support Object.getOwnPropertyDescriptors
Copy link
Contributor

@fatso83 fatso83 Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to fix the issues in #87 so this won't be needed.
Turns out that was the nise project. I'll update the Node versions here as well.

But thank you for good comments!

};
copyProps(window, global);

withGlobal = lolex.withGlobal(global);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fatso83 fatso83 merged commit 629ca1a into sinonjs:master Aug 4, 2019
@cdow cdow deleted the fix-jsdom-2 branch August 4, 2019 16:30
fatso83 added a commit that referenced this pull request Nov 9, 2021
Saw this was missing when reviewing DefinitelyTyped/DefinitelyTyped#56961

Bit funny that it has not been added until now, given it was added way back in #106 😄 (And later #169 #248 and others).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Cannot set property performance of #<Object> which has only a getter
3 participants