bcherny / json-schema-to-typescript

Compile JSON Schema to TypeScript type declarations
https://bcherny.github.io/json-schema-to-typescript-browser/
MIT License
2.94k stars 391 forks source link

Maximum call stack size exceeded #35

Closed donaldpipowitch closed 8 years ago

donaldpipowitch commented 8 years ago

Try to compileFromFile this schema throws.

/Users/foo/webpack/node_modules/lodash/lodash.js:2591
    function baseClone(value, isDeep, isFull, customizer, key, object, stack) {
                      ^

RangeError: Maximum call stack size exceeded
    at baseClone (/Users/foo/webpack/node_modules/lodash/lodash.js:2591:23)
    at /Users/foo/webpack/node_modules/lodash/lodash.js:2644:34
    at arrayEach (/Users/foo/webpack/node_modules/lodash/lodash.js:520:11)
    at baseClone (/Users/foo/webpack/node_modules/lodash/lodash.js:2638:7)
    at baseMergeDeep (/Users/foo/webpack/node_modules/lodash/lodash.js:3627:24)
    at /Users/foo/webpack/node_modules/lodash/lodash.js:3562:11
    at arrayEach (/Users/foo/webpack/node_modules/lodash/lodash.js:520:11)
    at baseMerge (/Users/foo/webpack/node_modules/lodash/lodash.js:3555:7)
    at /Users/foo/webpack/node_modules/lodash/lodash.js:13254:7
    at Function.<anonymous> (/Users/foo/webpack/node_modules/lodash/lodash.js:4781:13)

The webpack schema is quite heavy. Perfect stress test to optimize ;) Note that you have to delete this enum because of https://github.com/bcherny/json-schema-to-typescript/issues/34 to get this working for now.

darcyparker commented 8 years ago

An initial guess: https://github.com/bcherny/json-schema-to-typescript/blob/master/src/index.ts#L313 because merge() uses baseClone().

Is the full stack trace shown above? I didn't attempt to reproduce... but made this guess by looking for recursive lodash methods used by this project. My guess is https://github.com/bcherny/json-schema-to-typescript/blob/master/src/index.ts#L313 is in the stack trace.

I don't have time to look at this further today... and may not in next few days. But wanted to share my initial findings. I think https://github.com/bcherny/json-schema-to-typescript/blob/master/src/index.ts#L313 can be refactored to not use lodash and this may solve the problem.

donaldpipowitch commented 8 years ago

Is the full stack trace shown above?

Yes.

darcyparker commented 8 years ago

FYI: I have a test in place that blows the stack. https://github.com/darcyparker/json-schema-to-typescript/commit/db2b30c34fa3fce975c05a0c3b725b51c9dc3e1d

darcyparker commented 8 years ago

Updating link for line with merge() that I mentioned above because new commits changed the line number: https://github.com/bcherny/json-schema-to-typescript/blob/1aa73177ca6c3d0cf6963f9ff9e42e0ed57668cc/src/index.ts#L274 (This link points to a specific commit's file's line number)

darcyparker commented 8 years ago

A WIP that improves call stack usage is available for review. Please let me know if it helps.

bcherny commented 8 years ago

@donaldpipowitch You're getting a call stack exceeded error because you have a bunch of circular references in your schema

I'll add a circular reference checker to the ref resolver (the resolver is very bare bones as is). In the meantime, if you get rid of the circular references, your schema seems to compile fine.

donaldpipowitch commented 8 years ago

Thank you both. I'll close this issue than as it is probably a problem with the schema and its circular references.

donaldpipowitch commented 8 years ago

Short question: Is this really a circular reference? Doesn't this point to this definition?

darcyparker commented 8 years ago

Side question: Do you know of a tool that generates a graph of a json schema? (For example something that creates graphml for http://www.yworks.com/products/yed or http://www.cytoscape.org/, or alternatively dot files for http://graphviz.org/ dot) If not, I am thinking this could be an interesting project. It's one thing to write circular reference detection and give feedback when you walk to a node that you've been to before... but visually seeing it in a picture could be helpful.

bcherny commented 8 years ago

@donaldpipowitch You're right - I misread the schema. This issue is with resolving local $refs, not with a circular definition. Filed https://github.com/bcherny/json-schema-to-typescript/issues/44

@darcyparker Not off the top of my head, but it would be pretty trivial to write one I think.