Error when using window as a prop

[HiDeoo]

Do you want to request a feature or report a bug?

A bug.

What is the current behavior?

One third party React component I'm using use window as a prop. When trying to snapshot test it, I'm getting the following error:

RangeError: Invalid string length

      at Object.test (src/SelectField.test.js:17:52)
          at Promise (<anonymous>)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

I'm guessing this is an issue with pretty-format as the same error can be reproduced when trying to pretty format window:

test('something', () => {
  const windowRepresentation = prettyFormat(window)
});

This would lead to a similar error:

RangeError: Invalid string length

  at printListItems (node_modules/pretty-format/build/collections.js:174:256)
  at Object.<anonymous>.test (index.test.js:4:32)
      at Promise (<anonymous>)
      at <anonymous>
  at process._tickCallback (internal/process/next_tick.js:188:7)

I've setup a small repro leading to this exact same error when trying to pretty format window.

• jest version: 21.2.1
• node version: 8.4.0
• yarn version: 1.1.0

[pedrottimark]

@HiDeoo Thank you very much for the link. Here is my question and observation so far:

If you imagine an ideal solution, what would you like as value of that prop in a snapshot?

It looks window is alias for global in test file: expect(window === global).toBe(true)

• Off the main point, therefore it has: window.test window.expect and so on
• Possible solution: if pretty-format can detect that its arg is global or window without throwing ReferenceError in environments where they don’t exist, then it could return a specific string like [Window] instead of formatting all the properties?

[pedrottimark]

This reminds me that DOMElement plugin doesn’t know about document nodeType 9 either.

[HiDeoo]

I can confirm that window is indeed an alias for global. My current workaround for this issue is actually to mock and add a global.toJSON() to short-circuit pretty-format and avoid using printComplexValue().

I'm also as a matter of fact returning a [Window] string temporarily in my workaround so I think your idea to detect if it's window and then return a pre-defined string rather than trying to format it sounds pretty good. I can't really see the whole point to format it vs returning a string but I may be missing something.

[pedrottimark]

@HiDeoo You would smile to see time-lapse scan of my brain like Pooh Bear: think, think, think.

Before I make pull request for issue, can you share your thoughts about this candidate solution:

If global === window (lazy eval with try-catch) then if pretty-format arg is equal, return [Window] (that is, constructor name). Added to condition that stops serializing at max depth.

• Jest default --env=jsdom has global and window so it fixes the situation you reported.
• Browser script has window but not global so theoretically it could serialize genuine window successfully (less than 10K chars in Chrome and Firefox)
• However, require browser build of pretty-format in bundle via webpack defines global to equal window so it behaves like test environment.
• Generic node does not have window so no change for non-test use of package.

[HiDeoo]

Hi, thanks a lot for looking into this and also thinking about all possibles outcomes.

From my understanding of your proposal, it should indeed fix the issue no matter the environment so we would get consistent results without any error. I would assume genuinely serializing window on different browsers will produce different results.

I should even be able to test the PR locally on my previous code having this issue, remove my temporary workaround and should get green tests without any difference in the snapshots.

[pedrottimark]

Yes, the results did differ.

Thanks if you can test #4750 locally. Your code review is welcome too :)

[HiDeoo]

Just tested your PR, after commenting out my workaround, it was a drop in solution and every tests passed without having to update any snapshots 🎉 .

Regarding the code evolution in the PR and what you said above, I think it's better indeed to not consider global at all, it makes things clearer in my opinion.

Thanks for your amazing work.

[github-actions]

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.