250 with recursion limiter #277
Conversation
…oving console.log
|
@bmuenzenmeyer Please review. Includes your unit test from branch 250. |
|
@bmuenzenmeyer @geoffp This is currently a Mustache/Handlbars-only solution. It escapes partial includes from rendering by temporarily switching them to ERB syntax. I think it would be trivial to extend this functionality to other templating languages, assuming we don't provide an ERB option. And even if we do, it would be trivial to escape ERB tags as well. |
|
@e2tha-e I did a quick read through - thanks for the thorough, thoughtful improvement. I can't wait to review this! |
…on paths in parameter_hunter.js
e2tha-e
changed the title
|
@bmuenzenmeyer I'm looking into optimizing this further based on feedback from @vebersol in issue #250 . Feel free to review, but do not merge for the time being. |
e2tha-e
changed the title
…rnlab-node into 250-with-recursion-limiter
e2tha-e
changed the title
|
@bmuenzenmeyer This PR is ready for review now. It has the latest from dev merged in, and all merge conflicts resolved. Large source directories that I have from Pattern Lab PHP can be dropped in and everything just works. |
|
What a phenomenal amount of work. It looks good, but there's so much that I admit I have a hard time understanding it well enough to reason about it. But I'm trying. The main thing I'm afraid of here is that the main line of development is now on the dev-2.0 branch, which is where the pattern engines are, and it because this has lots of mustache-specific stuff in it (I think), it's gotta get into that branch somehow. @e2tha-e, would you investigate what it would take to rebase this on dev-2.0? |
|
I echo @geoffp's admiration and concern. It'd be a big win to get this into Pattern Lab - as you've invested a lot into improving the performance of processing patterns. If rebasing to |
|
@geoffp @bmuenzenmeyer I read you loud and clear. I'll definitely make the effort to rebase into In the meantime, this branch has worked very well as a stopgap for the enterprise Pattern Lab work I've been doing. The real-world use-case for Pattern Lab Node over Pattern Lab PHP is for Node applications to instantiate a Pattern Lab object like any other Node object. Hopefully this can be useful for others as well. |
|
@e2tha-e that's true. We are moving to support (we call it "pattern engines") multiple templating engines. Pattern parameters exist only to bolt features on to Mustache that are provided natively by other engines, so long or medium-term, I'd like to see all of pattern parameters disappear into the Mustache engine where it can't bother anyone else. I'd love to know more about what your enterprise use case is for PL! |
|
I should also warn you: dev-2.0 is pretty torn up at the moment. We're also splitting it apart into separate composable repos like the PHP version. I'll write some documentation and put it up on the READMEs. |
…-with-recursion-limiter
|
@geoffp About the enterprise Pattern Lab implementations, I created tool called Fepper (a portmanteau of Front End Prototyper). This started out being a series of common tasks most projects would need in order to import Pattern Lab patterns into a backend, in our case, Drupal. However, more tools got added to Fepper over time, like the separation and compilation of partial JSON files for large global data sets. Fepper originally started out using Pattern Lab PHP. The first enterprise project I used Pattern Lab PHP and Fepper for comprised 268 Mustache files. I'm currently working on a project that will likely comprise over a thousand. So why is Pattern Lab Node better for such a large codebase than PHP? It's because since Fepper tasks are run in Node.js, it would be advantageous to instantiate Pattern Lab objects in Node.js. Fepper is built to be extended, and one of the extensions is Multisite functionality. The Multisite extension allows any number of Pattern Lab instances to be instantiated, built, and viewed within a single UI. I originally envisioned the use-case to be when building many sites that were variations on a theme. However, the actual use-case has turned out to be a way to limit expensive builds. For example, if we have 10 subsites, each with 100 patterns, for a given site, we only need to turn on the primary site and 1 subsite. This means only 200 patterns will build, instead of all 1000. It's unlikely a developer would need all subsites enabled for any given development task. |
|
@e2tha-e Pattern Lab 2.0.0-alpha is on |
|
@bmuenzenmeyer I am definitely planning on working on this. I'm just trying to get the usual set of other priorities out of the way. I'm also sure you'd be interested to know that I have been asked to speak on Pattern Lab for Node.js at Node Camp in NYC this July at the UN. http://nodecamp.io/ . I accepted :-) |
Great to hear. I hope that the new structure is more clear, but let me know if you run into any snags along the way.
Wow!!!!! Can't wait to here more about it. |
|
@bmuenzenmeyer I've got a bunch of things out of the way, so I can take a look at this now. I'll have to take some time to get my bearings on the current state of things, but I'm definitely looking forward to making progress on this. |
|
@e2tha-e great to hear. This PR's code may still be relevant, but I'd like to avoid merging in so many commits. Whatever we can do to make this several smaller, reviewable chunks I encourage it. The codebase has shifted out from under the PR a bit too, with a lot more emphasis on modular components. I think that a bulk of code necessary will live in https://github.com/pattern-lab/patternengine-node-mustache as well as core - take some time to read through how the core engine code works with abstractions in the Pattern Object and please ask me or @geoffp if you have questions. Geoff has even started a handlebars engine which goes a long way toward obsoleting some mustache-only syntax hurdles. Perhaps the hogan work you've done here should have its own engine too. |
|
also - feel free to hang out in https://gitter.im/pattern-lab/node where we can all chat a bit more simply |
|
Please see #603 for an explanation of why I am closing this PR. |
Addresses #250
Summary of changes:
Adds recursion to the findparameters() function. However, it also limits recursion by immediately rendering non-partial Mustache tags. So if a parameter had been submitted to performed the Mustache partial inclusion, the inclusion, and therefore the recursion, would be performed, but not otherwise. This gives users the ability to code many levels of optional inclusions, but without forcing Pattern Lab to recurse every single possible option, which without the limit, would grow exponentially per level.