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

TypeError: cyclic object value with JSON.stringify #4910

Open
3 tasks done
jmlopez-rod opened this issue Aug 10, 2022 · 4 comments
Open
3 tasks done

TypeError: cyclic object value with JSON.stringify #4910

jmlopez-rod opened this issue Aug 10, 2022 · 4 comments
Labels
status: in discussion Let's talk about it! type: bug a defect, confirmed by a maintainer

Comments

@jmlopez-rod
Copy link

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • [] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Mocha uses JSON.stringify without a replacer when dealing with objects that have circular references in the browser.

Steps to Reproduce

See https://github.com/jmlopez-rod/mocha-stringify-issue

Seems that react elements have cyclic references. I was able to fix the issue by overriding the definition of JSON.stringify so
that it uses a replacer to deal with cyclic references following the example in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value

Expected behavior: mocha should not be stuck trying to stringify react elements.

fail-fast

The above screenshot includes a simple quick fix to make it fail fast.

Actual behavior: In the browser we can see where the error is coming from

error

The error is

Uncaught (in promise) TypeError: cyclic object value

disconnected

Due to this error we can see that karma disconnects.

Reproduces how often: Every run I've made in a mac and linux I see the issue.

Versions

  • The output of mocha --version and node_modules/.bin/mocha --version:

$ npx mocha --version
10.0.0

  • The output of node --version:
  • Your operating system
    • name and version: Fedora
    • architecture (32 or 64-bit): 32
  • Your shell (e.g., bash, zsh, PowerShell, cmd): bash
  • Your browser and version (if running browser tests):

$ firefox --version
Mozilla Firefox 85.0

  • Any third-party Mocha-related modules (and their versions):
    "@babel/core": "^7.18.10",
    "@babel/preset-env": "^7.18.10",
    "@babel/preset-react": "^7.18.6",
    "@testing-library/react": "^13.3.0",
    "babel-loader": "^8.2.5",
    "chai": "^4.3.6",
    "chai-dom": "^1.11.0",
    "karma": "^6.4.0",
    "karma-firefox-launcher": "^2.1.2",
    "karma-mocha": "^2.0.1",
    "karma-mocha-reporter": "^2.2.5",
    "karma-webpack": "^5.0.0",
    "mocha": "^10.0.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "webpack": "^5.74.0"
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version):

Additional Information

See the files in the repo with the reproduction of the issue

https://github.com/jmlopez-rod/mocha-stringify-issue

For now the workaround I use is to monkeypatch JSON.stringify when running tests.

@cherniavskii
Copy link

I've experienced this issue as well in https://github.com/mui/mui-x (mui/mui-x#5851 (comment) specifically)
Mocha's stringify helper cannot stringify an HTML element with a circular reference, here's a minimal example to reproduce:

const element = document.createElement('div');
element.reference = element;
stringify(element); // will throw

But I wonder if that can be avoided on karma-mocha's end:
It calls stringify on expected and actual:
https://github.com/karma-runner/karma-mocha/blob/master/src/adapter.js#L57-L58

Removing window.Mocha.utils.stringify from lines above seems to fix the issue, and frankly I don't understand why it's needed there in the first place.

So I see two options here:

  1. do not call stringify on HTML Elements in karma-mocha
  2. support HTML elements with circular references in mocha's stringify

@oliviertassinari
Copy link

I would vote for 1. The logic was introduced in karma-runner/karma-mocha#85 for being able to tap into internal APIs, but it seems that not even the author of the PR needs karma-mocha to stringify the expected and actual keys, he could do this on his own.

@JoshuaKGoldberg
Copy link
Member

Hmm, HTML elements aren't going to be the only cyclic object values that break Mocha in this way. Synthetic React elements per the OP are an immediate other example.

I think @jmlopez-rod's solution of manually adding in cycle detection for JSON.stringify makes sense to me. As in, Mocha could provide a replacer per the JSON.stringify API.

Copying code from the mocha-stringify-issue repo
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value
const getCircularReplacer = () => {
  const seen = new WeakSet();
  return (key, value) => {
    if (typeof value === "object" && value !== null) {
      if (seen.has(value)) {
        return "[seen]";
      }
      seen.add(value);
    }
    return value;
  };
};

const jsonStringify = JSON.stringify;

// Uncomment so that mocha may use this definition which will avoid the
// infinite loop trying to stringify the react element.

JSON.stringify = (val) => jsonStringify(val, getCircularReplacer, 2);

cc @mochajs/maintenance-crew just to be thorough - what do you think?

@JoshuaKGoldberg JoshuaKGoldberg added type: bug a defect, confirmed by a maintainer status: in discussion Let's talk about it! and removed unconfirmed-bug labels Jan 15, 2024
@voxpelli
Copy link
Member

@JoshuaKGoldberg I would go with an established library like fast-safe-stringify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: in discussion Let's talk about it! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

5 participants