handling circular/shared structures in deep-clone

[azizghuloum]

Q	A
Fixed Issues?	unreported issue
Patch: Bug Fix?	yes
Major: Breaking Change?	no
Minor: New Feature?	no
Tests Added + Pass?	no
Documentation PR Link	no
Any Dependency Changes?	no
License	MIT

Compiling our project gets into deep recursion and eventually stack overflow inside deepClone. Setting the cache early (as in the patch) cuts on the recursion in many cases and actually made our project compile.

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53903/

[liuxingbaoyu]

Thank you for your PR! Can you give a concrete example? This fix works fine for me, but in practice the AST should not have circular references.

[azizghuloum]

Correct. It's bizarre that it crashed there and more bizarre that this fixes it. What was crashing on it was react-native-code-push as part of its bundling task, and honestly I don't know of its internals or how it's using deepClone. All I know is that after upgrading some dependencies, our pipeline started failing and after digging I found it in deepClone. It could be that it's used outside of its intended use. Dunno honestly.

[liuxingbaoyu]

Well, there may be some plugin storing other data on the ast, which seems a bit dangerous.

[azizghuloum]

Agreed. If I find out who's doing this, I'll let them know 😄

Thanks for the quick approval. 👍

[nicolo-ribaudo]

Thank you! Could you add this test?

  it("transformFromAstSync should not cause infinite recursion with circular objects", () => {
    const program = "const identifier = 1";
    const node = parseSync(program);
    node.program.body[0].extra = { parent: node.program };

    expect(transformFromAstSync(node, program, {}).code).toBe(
      "const identifier = 1;",
    );
  });

in babel-core/test/api.js (there are already other tests for transformFromAstSync).