rocjs / roc

🐦 Modern JavaScript Development Ecosystem
MIT License
425 stars 23 forks source link

possible Handlebars dependency collision #170

Closed kjarmicki closed 7 years ago

kjarmicki commented 7 years ago

I've ran into an unusual issue and I'd like to share my findings and possible solution :)

Problem: If Roc Template has Handlebars in it's dependencies then helpers are not reachable.

How to reproduce: Create a Roc Template that has Handlebars in it's dependencies and use {{#if_eq}} somewhere in that template -> it'll say

✖ An error happened when creating the template

Missing helper: "if_eq"
A problem occurred when running the command

Why this happens: if Handlebars are declared in Template dependencies, they're going to be directly under node_modules with other dependencies (level 1): screen shot 2017-04-26 at 14 17 21

But installed Roc has it's own Handlebars in it's node_modules, possibly due to differences in version (level 2): screen shot 2017-04-26 at 14 21 31

Here's where it all collapses: Roc uses Consolidate to render templates, and inside it's requiring Handlebars ( https://github.com/tj/consolidate.js/blob/master/lib/consolidate.js#L726 ) -> but since Consolidate is on level 1 it's getting different Handlebars than Roc, which gets Handlebars from it's own node_modules, therefore helpers are set on Roc's Handlebars but not on Consolidate's Handlebars.

Proposed solution: either do not use Consolidate and just do Handlebars.compile() directly or inject helpers into Consolidate's render function: https://github.com/tj/consolidate.js/blob/master/lib/consolidate.js#L731

I'd like to discuss which solution would make more sense and I can implement it. Personally I'm leaning towards getting rid of Consolidate (one less dependency, yay) and using Handlebars directly.

dlmr commented 7 years ago

I think removing consolidate is the best approach her. 👍

kjarmicki commented 7 years ago

Cool :) PR coming soon