metalsmith / layouts

A metalsmith plugin for layouts
MIT License
116 stars 49 forks source link

Fails with infinite recursion if passing a recursive object to `locals` #83

Closed emirotin closed 8 years ago

emirotin commented 8 years ago

I have an object (navigation tree, with parent -> children -> child -> parent references) passed to locals. It causes building failing with what's most likely a call stack overflow. The stack looks like

  [...skipped]
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at extend (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/extend/index.js:72:22)
  at convert (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/lib/index.js:120:26)
  at /Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/async/lib/async.js:181:20
  at Object.async.forEachOf.async.eachOf (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/async/lib/async.js:233:13)
  at async.forEach.async.each (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/node_modules/async/lib/async.js:209:22)
  at Ware.<anonymous> (/Users/admin/Work/resin-docs/node_modules/metalsmith-layouts/lib/index.js:150:5)
  at Ware.<anonymous> (/Users/admin/Work/resin-docs/node_modules/metalsmith/node_modules/ware/node_modules/wrap-fn/index.js:45:19)
  at Immediate.next (/Users/admin/Work/resin-docs/node_modules/metalsmith/node_modules/ware/lib/index.js:85:20)
  at Immediate._onImmediate (/Users/admin/Work/resin-docs/node_modules/metalsmith/node_modules/ware/node_modules/wrap-fn/index.js:121:18)
  at processImmediate [as _immediateCallback] (timers.js:383:17)

Confirmed by removing backlinks (not a huge deal because of that).

ismay commented 8 years ago

I assume you're using jade because you mention using locals? Just from this output it is difficult to see whether this is a bug in metalsmith-layouts, or a configuration error.

If you could create a reduced test case that would help in troubleshooting this.

emirotin commented 8 years ago

I use swig. Thanks, I will try to repro it.

emirotin commented 8 years ago

So it's actually simple.

Create any recursive object:

nav = {
  title: "root",
  children: [ { title: "child" } ]
}
nav.children[0].parent = nav

Then pass locals: { nav: nav }

When you call var clonedParams = extend(true, {}, params); it goes into infinite recursion.

ismay commented 8 years ago

Hi @emirotin, what I meant with reduced test case was a repo that replicates the problem in the most minimal way possible. It is difficult for me to judge what's going on from code snippets, I need to see your entire config.

emirotin commented 8 years ago

I already reduced it to the specific line of code in your module (var clonedParams = extend(true, {}, params); is taken straight from index.js). It's already minimal because it's isolated from the rest of params, template language, and everything. And I also explained what happens there: the module you use for deep objects cloning (extend) can't deal with circular refs.

ismay commented 8 years ago

You're giving me the problem, but not a complete example of the use case that leads to that problem. If you want me to take a look at it I'll need the complete picture.

I'll close this for now. Feel free to point me at an example repo if you have one and I'll be happy to take a look. Otherwise I'll see if I can dive into this when I have the time.