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

Pretty-format toJSON maximum call stack error #4956

Closed
NickMalt opened this issue Nov 25, 2017 · 7 comments · Fixed by #5044
Closed

Pretty-format toJSON maximum call stack error #4956

NickMalt opened this issue Nov 25, 2017 · 7 comments · Fixed by #5044
Assignees

Comments

@NickMalt
Copy link

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

What is the current behavior?
As I understand this code expect that toJson function return already stringified version of object, which is not right. And eventually this leads to Maximum call stack error since printing of serializable object is not handled after toJSON call. Or maybe I miss something?

If the current behavior is a bug, please provide the steps to reproduce and
either a repl.it demo through https://repl.it/languages/jest or a minimal
repository on GitHub that we can yarn install and yarn test.

Debugging gif

What is the expected behavior?
handle serializable object printing after a toJSON call

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

jest: 21.2.1
node: 9

@SimenB
Copy link
Member

SimenB commented Nov 25, 2017

From what I can understand in the docs, toJSON is expected to always return a string - just not necessarily something that can be fed to JSON.parse. Or am I misreading it?

/cc @pedrottimark

@cpojer
Copy link
Member

cpojer commented Nov 25, 2017

JSON is a string representation of JavaScript objects. toJSON implies that you are creating that string representation. If you are returning something that is not a string it is by definition not JSON and means your API is wrongly named or doing the wrong thing.

@AntonPuko
Copy link

AntonPuko commented Nov 27, 2017

@cpojer toJSON() is kind of middlware for JSON.stringify() transformation, and it shouldn't always return string, but anything that can be passed directly to JSON.stringify().
just look at this example:
https://docs.microsoft.com/en-us/scripting/javascript/reference/json-stringify-function-javascript#example-2

@cpojer
Copy link
Member

cpojer commented Nov 27, 2017

Yap, you are right, my mistake. cc @pedrottimark

@cpojer cpojer reopened this Nov 27, 2017
@Qix-
Copy link

Qix- commented Nov 27, 2017

I think @AntonPuko outlined some simple reproduction steps in qix-/color#134 (mentioned above).

@pedrottimark
Copy link
Contributor

pedrottimark commented Dec 8, 2017

@NickMalt @AntonPuko Hello, friends. pretty-format teaches me more and more JavaScript :)

There is a unit test in which pretty-format serializes an object returned by toJSON method: https://github.com/facebook/jest/blob/master/packages/pretty-format/src/__tests__/pretty_format.test.js#L663-L670

The problem is that pretty-format supports calling toJSON recursively EDIT CORRECT LINK: https://github.com/facebook/jest/blob/master/packages/pretty-format/src/__tests__/pretty_format.test.js#L690-L697

Because Color.prototype.toJSON returns an instance of Color which has a toJSON method, it’s colors all the way down to Maximum call stack size exceeded /cc @Qix- in case you are interested

By definition, JSON.stringify ignores prototype properties and own methods of an object.

Question for you and my fellow collaborators: can you think of a use case for pretty-format to call toJSON recursively? Or do you think the feature “fell out of” the implementation?

A tentative local change breaks the one unit test and formats the pretty Jest color as follows:

const Color = require('color');
const format = require('pretty-format');
format(Color.rgb(153, 66, 91));
/*
Object {
  "color": Array [
    153,
    66,
    91,
  ],
  "model": "rgb",
  "valpha": 1,
}
*/

@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 13, 2021
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.

6 participants