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

Cannot assign to read only property '...' of object '...' #10408

Closed
chrisbobbe opened this issue Aug 14, 2020 · 10 comments · Fixed by #10414
Closed

Cannot assign to read only property '...' of object '...' #10408

chrisbobbe opened this issue Aug 14, 2020 · 10 comments · Fixed by #10414

Comments

@chrisbobbe
Copy link

chrisbobbe commented Aug 14, 2020

🐛 Bug Report

Manually transferring facebook/react#18758 to here. During debugging, it was determined to be a Jest issue.

@gaearon rightly agreed with me that the issue won't get fixed by sitting in the React repo. But they used their facebook admin powers to lock the issue, instead of using GitHub's "Transfer Issue" feature to get it here with all its detail in an instant. Ah, well.

People (including React repo maintainers, apparently) are seeing variations on TypeError: Cannot assign to read only property 'Symbol(...)' of object '[object ...]' when they do expect(...).toBe(...) and expect(...).toEqual(...).

The initial response is that it's not a very helpful error message.

Here's one clue that might explain it:

It happens when calling replaceMatchedToAsymmetricMatcher() inside printDiffOrStringify()

https://github.com/facebook/jest/blob/master/packages/jest-matcher-utils/src/index.ts#L359

@SimenB any ideas?

Originally posted by @pahan35 in facebook/react#18758 (comment)

And a suggestion about what should be done about it:

We see a current message because replaceMatchedToAsymmetricMatcher fails on preparing the diff.

IMHO it's not about improving the error message, it's about fixing replaceMatchedToAsymmetricMatcher to print the diff for DOM elements without failures.

Originally posted by @pahan35 in facebook/react#18758 (comment)

@gaearon
Copy link
Contributor

gaearon commented Aug 15, 2020

Hmm I didn’t even know there’s a way to transfer issues. 😀

@SimenB
Copy link
Member

SimenB commented Aug 15, 2020

@WeiAnAn
Copy link
Contributor

WeiAnAn commented Aug 15, 2020

Sorry guys.
I'm trying to figure out the problem and solve it.

@WeiAnAn
Copy link
Contributor

WeiAnAn commented Aug 15, 2020

Found problem at https://github.com/facebook/jest/blob/c9c8dba4dd8de34269bdb971173659399bcbfd55/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts#L58-L74

We need to set symbol key property descriptor.

But I got error on typescript that symbol type cannot be object key.
An index signature parameter type must be either 'string' or 'number'.

Will continue work tomorrow.

If anyone interested in, welcom to take it.

@ahnpnl
Copy link
Contributor

ahnpnl commented Aug 15, 2020

Found problem at

https://github.com/facebook/jest/blob/c9c8dba4dd8de34269bdb971173659399bcbfd55/packages/jest-matcher-utils/src/deepCyclicCopyReplaceable.ts#L58-L74

We need to set symbol key property descriptor.

But I got error on typescript that symbol type cannot be object key.
An index signature parameter type must be either 'string' or 'number'.

Will continue work tomorrow.

If anyone interested in, welcom to take it.

microsoft/TypeScript#1863 Probably related to the error

@WeiAnAn
Copy link
Contributor

WeiAnAn commented Aug 16, 2020

Here is the minimal to reproduce this error.

test('test', () => {
  const a = {};
  const b = {};
  const symbolKey = Symbol.for('key');
  Object.defineProperty(a, symbolKey, {
    configurable: true,
    enumerable: true,
    value: {
      a: 1,
    },
    writable: false,
  });
  Object.defineProperty(b, symbolKey, {
    configurable: true,
    enumerable: true,
    value: {
      a: 1,
    },
    writable: false,
  });
  expect(a).toBe(b);
});

@SimenB
Copy link
Member

SimenB commented Aug 16, 2020

@WeiAnAn we can do a // @ts-expect-error to suppress it - better than failing at runtime 🙂

@SimenB
Copy link
Member

SimenB commented Aug 20, 2020

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Aug 21, 2020
In particular, this gets us Jest 26.4.1, which contains
jestjs/jest#10414. This should fix an issue [1] we'd otherwise
encounter very soon, where differences in a URL object encountered
with `expect(...).toBe(...)` or `expect(...).toEqual(...)` would
result in a very cryptic error message instead of a straightforward
test failure with the desired explanation.

This upgrade was pretty easy to do! I'm hoping fb23341 has
increased the ease of staying up-to-date with Jest.

Since we're going from a 22.x to 23.x of eslint-plugin-jest, we
check the declared breaking changes at 23.0.0 [2]. The only one that
concerns us is that part of the job of `jest/valid-describe` has
been transferred to `jest/valid-title`, and `jest/valid-title` has
been added to the `jest/recommended` config, which we take. The
validation in question is silly, and unfortunately, we can't
reasonably take all of `jest/valid-title` except that part, so, turn
it all off.

See also some prep work for this upgrade in a recent commit.

[1] jestjs/jest#10408
[2] https://github.com/jest-community/eslint-plugin-jest/releases/tag/v23.0.0
@chrisbobbe
Copy link
Author

That's awesome, thanks a lot for the fix, @WeiAnAn and @SimenB! 😄

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 16, 2020
In particular, this gets us Jest 26.4.1, which contains
jestjs/jest#10414. This should fix an issue [1] we'd otherwise
encounter very soon, where differences in a URL object encountered
with `expect(...).toBe(...)` or `expect(...).toEqual(...)` would
result in a very cryptic error message instead of a straightforward
test failure with the desired explanation.

This upgrade was pretty easy to do! I'm hoping fb23341 has
increased the ease of staying up-to-date with Jest.

Since we're going from a 22.x to 23.x of eslint-plugin-jest, we
check the declared breaking changes at 23.0.0 [2]. The only one that
concerns us is that part of the job of `jest/valid-describe` has
been transferred to `jest/valid-title`, and `jest/valid-title` has
been added to the `jest/recommended` config, which we take. The
validation in question is silly, and unfortunately, we can't
reasonably take all of `jest/valid-title` except that part, so, turn
it all off.

See also some prep work for this upgrade in a recent commit.

[1] jestjs/jest#10408
[2] https://github.com/jest-community/eslint-plugin-jest/releases/tag/v23.0.0
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 17, 2020
In particular, this gets us Jest 26.4.1, which contains
jestjs/jest#10414. This should fix an issue [1] we'd otherwise
encounter very soon, where differences in a URL object encountered
with `expect(...).toBe(...)` or `expect(...).toEqual(...)` would
result in a very cryptic error message instead of a straightforward
test failure with the desired explanation.

This upgrade was pretty easy to do! I'm hoping fb23341 has
increased the ease of staying up-to-date with Jest.

Since we're going from a 22.x to 23.x of eslint-plugin-jest, we
check the declared breaking changes at 23.0.0 [2]. The only one that
concerns us is that part of the job of `jest/valid-describe` has
been transferred to `jest/valid-title`, and `jest/valid-title` has
been added to the `jest/recommended` config, which we take. The
validation in question is silly, and unfortunately, we can't
reasonably take all of `jest/valid-title` except that part, so, turn
it all off.

See also some prep work for this upgrade in a recent commit.

[1] jestjs/jest#10408
[2] https://github.com/jest-community/eslint-plugin-jest/releases/tag/v23.0.0
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 18, 2020
In particular, this gets us Jest 26.4.1, which contains
jestjs/jest#10414. This should fix an issue [1] we'd otherwise
encounter very soon, where differences in a URL object encountered
with `expect(...).toBe(...)` or `expect(...).toEqual(...)` would
result in a very cryptic error message instead of a straightforward
test failure with the desired explanation.

This upgrade was pretty easy to do! I'm hoping fb23341 has
increased the ease of staying up-to-date with Jest.

Since we're going from a 22.x to 23.x of eslint-plugin-jest, we
check the declared breaking changes at 23.0.0 [2]. The only one that
concerns us is that part of the job of `jest/valid-describe` has
been transferred to `jest/valid-title`, and `jest/valid-title` has
been added to the `jest/recommended` config, which we take. The
validation in question is silly, and unfortunately, we can't
reasonably take all of `jest/valid-title` except that part, so, turn
it all off.

See also some prep work for this upgrade in a recent commit.

[1] jestjs/jest#10408
[2] https://github.com/jest-community/eslint-plugin-jest/releases/tag/v23.0.0
chrisbobbe added a commit to zulip/zulip-mobile that referenced this issue Sep 18, 2020
In particular, this gets us Jest 26.4.1, which contains
jestjs/jest#10414. This should fix an issue [1] we'd otherwise
encounter very soon, where differences in a URL object encountered
with `expect(...).toBe(...)` or `expect(...).toEqual(...)` would
result in a very cryptic error message instead of a straightforward
test failure with the desired explanation.

This upgrade was pretty easy to do! I'm hoping fb23341 has
increased the ease of staying up-to-date with Jest.

Since we're going from a 22.x to 23.x of eslint-plugin-jest, we
check the declared breaking changes at 23.0.0 [2]. The only one that
concerns us is that part of the job of `jest/valid-describe` has
been transferred to `jest/valid-title`, and `jest/valid-title` has
been added to the `jest/recommended` config, which we take. The
validation in question is silly, and unfortunately, we can't
reasonably take all of `jest/valid-title` except that part, so, turn
it all off.

See also some prep work for this upgrade in a recent commit.

[1] jestjs/jest#10408
[2] https://github.com/jest-community/eslint-plugin-jest/releases/tag/v23.0.0
@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 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants