Skip to content
This repository has been archived by the owner. It is now read-only.

Unify Pattern Lab Core #37

Closed
bmuenzenmeyer opened this issue Jan 12, 2018 · 31 comments
Closed

Unify Pattern Lab Core #37

bmuenzenmeyer opened this issue Jan 12, 2018 · 31 comments
Assignees
Labels

Comments

@bmuenzenmeyer
Copy link
Member

@bmuenzenmeyer bmuenzenmeyer commented Jan 12, 2018

Okay, here goes.

TL;DR Officially declare the authoritative representation of the Pattern Lab spec to be Pattern Lab Node. Signal our intent to deprecate and place Pattern Lab PHP to be under community maintenance only.

Since Pattern Lab was a scrappy proof of concept that @bradfrost created many years ago, the specification of what Pattern Lab is and isn't has been represented by the functionality implemented within the PHP application. Dave and Brad quickly turned around new features, responding to needs of the community. When Pattern Lab Node came along, I had a truly great living reference to build toward.

Cue Pattern Lab 2.0 🎉

With 2.0 came the ecosystem, an architecture and separation of concerns that powered our cross-platform efforts for going on 2 years now. Dave wrote the spec, which captures some of the essence of Pattern Lab's features - but I'll admit, in practice, the most current version of Pattern Lab PHP has always been what I reference as the authoritative spec.

I've spent hours comparing output between PHP and Node, usually to better understand where I need to go on the Node side. This process led to broad feature parity (though probably not complete), with updates and more movement coming out of Node. I've had to compare output less and less, excepting nuanced use-cases that required a suitable cross-platform acid-test. This process, while fruitful, is still slow. Effort within core and the styleguide front end forever need to be made in synchronicity with both platforms. It's a burden on maintainers and our users to compare and contrast versions, even forced to make hard decisions about converting from one to the other due to the issue or workaround of the hour.

Substantive changes to the core engine are proposed and implemented with a reasonable degree of success by utilizing this very spec repo - but the process is still duplicative. People have noted the irony in Pattern Lab, a tool to help make DRY componentry, offers a fragmented experience. This makes us slower and again harder for the community to know what features are in what platform.

What I propose is this. Deprecate Pattern Lab PHP. Unify the core engine under the Node repository.

Pattern Lab Node 3.0, once released, is positioned to be much more than it's predecessor.

  • Pattern Lab Node can run standalone, without the need for a task runner like gulp or grunt
  • Asynchronous rendering opens up more first-class support for PatternEngines like React 👀 , Liquid, even Twig.
  • Ever-increasing unit test support brings more confidence to the end-user and contributors
  • Improve documentation and configuration across the board

I'm excited about all of this - and what comes after these capabilities are present. We have big plans. The benefits of unifying core are broad. For our users, our documentation, our workflow, and our product.

  • Pattern Lab output need not be laboriously cross-checked between two separate systems
  • Maintainers can respond quicker to the needs of the community
  • New features from contributors do not need to be labeled or cross-checked as "on" or "off" spec.
  • Major architectural refactoring can occur across a smaller footprint
  • Simplify the documentation
  • Unify the communities
  • Create a single entry point into the ecosystem
  • Users get a clearer sense of what features Pattern Lab offers

The way forward

I don't expect this issue to completely answer all the questions that may arise from this effort. The initial outline of work is tangible, however. If approved, I plan to break out a bunch of milestones.

  • Announce intent (this issue)
  • Audit and catalog differences between both platforms - create issues within Pattern Lab Node where needed
  • Create starterkit-mustache-spec - a combination demo / acid-test that demonstrates Pattern Lab features succinctly. This won't be the polished web design project like the current demo is. Rather, it will be a terse set of patterns that in isolation (or together, where needed) demonstrate current behavior. It's my hope to build this, automate its build, test against it during CI/CD, and automatically host results, also via CI/CD
  • Update all documentation
  • Invest in more tooling to make collaboration more possible. Think prettier, standard-version, CI/CD integration tests, more people releasing
  • Stabilize and release Pattern Lab Node 3.0
    • I understand there are things about Pattern Lab Node that are sub-optimal, broken, or "off-spec". Let's fix em!

I think the above effort, and honestly this entire decision, will help in these ongoing or proposed efforts:

All of this won't happen overnight. Believe me - I understand how time is tight. But discussing this decision and communicating intent is the first step. So here we are!

@pattern-lab/voting-members

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 12, 2018

My vote: 👍

@gjhead
Copy link

@gjhead gjhead commented Jan 12, 2018

A++

@ThePeach
Copy link

@ThePeach ThePeach commented Jan 12, 2018

👍 good to hear this. Approved :)

@bradfrost
Copy link
Member

@bradfrost bradfrost commented Jan 12, 2018

Big 👍 vote from me!

The only thing that I feel is absent is a lot of the discussion with @EvanLovely about having some semblance of a true PHP engine in order to ensure things like twig (not twig.js) renders properly. My understanding of that is that there would need to be some sort of a lightweight PHP engine that handles that part of the puzzle, but everything else would happen through Node. Do I have that right?

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 12, 2018

Yes @bradfrost you are right - the existing Twig solutions are not adequate, and while I thought one of them did in fact delegate rendering to the actual PHP runtime, I will delegate judgement to someone more versed in PHP / Twig development.

Async rendering should enable the theoretical capability, but a true PHP-run Twig PatternEngine will be a new thing.

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 12, 2018

I think the problem is that there doesn't seem to be a proper spec, only a table of contents. And that is why the PHP version has so far been the de facto spec.

If Pattern Lab had a proper spec, detailing everything at the required level, and an acid test project covering the entire spec, I don't think there would be a need to declare an authoritative representation.

Do we see value in having a proper spec? Is it worth the extra work? Personally I think it depends on whether it is possible to have true Twig support for the Node version.

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 12, 2018

@aleksip right - the spec was only ever an outline - and something that would require herculean effort to flesh out in writing. That's why I mentioned creating a starterkit-mustache-spec. Mustache remains our default PatternEngine. Upgrading the default to Handlebars would actually do-away with a lot of cruft - but that's a different conversation!

Each engine needs to make a determination about what degree of the spec it supports, and how. Some engines are more capable than others (layout, extends) or do work differently (like the Handlebars PatternEngine ) than Mustache does. So, for the purposes of what is considered "spec" (especially in regards to template rendering), we are usually referring to Mustache.

No, I don't see value in having a true spec document. But I do see value in a hosted, terse, unit-testable starterkit-mustache-spec. That could even serve as the basis for deviations outlined within individual PatternEngine capabilities.

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 12, 2018

I don't think there would be a need to declare an authoritative representation.

The declaration is less important as is how this decision plays out - it signals that the core team will not be supporting PL PHP in earnest in the long term.

@bradfrost
Copy link
Member

@bradfrost bradfrost commented Jan 12, 2018

@bmuenzenmeyer I think you do what you need to do in order to make progress. It sounds like a spec would incur a lot of work, and I think that's time that would be better spent building real software.

@geoffp
Copy link

@geoffp geoffp commented Jan 12, 2018

👍 excellent.

@sghoweri
Copy link

@sghoweri sghoweri commented Jan 12, 2018

The only thing that I feel is absent is a lot of the discussion with @EvanLovely about having some semblance of a true PHP engine in order to ensure things like twig (not twig.js) renders properly. My understanding of that is that there would need to be some sort of a lightweight PHP engine that handles that part of the puzzle, but everything else would happen through Node. Do I have that right?

I'd love to hear @EvanLovely's thoughts on this but speaking for myself and the Bolt Design System team (plus my 2 cents from the Drupal Pattern Lab Working Group's POV), I've got a couple thoughts to add:

I'm 100% behind this initiative and direction we're taking, so long as the Pattern Lab PHP community isn't getting left behind here and getting adequately represented.

For me, this means you guys understanding and updating PL Node to fully support the still-growing ecosystem of CMS-enabled Design Systems, having native Twig PHP support, support for Twig extensions, support for Twig namespaces, support for plugins, etc out of the gate -- flagship advantages to the current PL PHP Core that we can't do our jobs (or power our "Holy Grail" Design Systems) without.

We need a solid plan in place to make any changes needed on the PHP and Node sides of the house to get us all living under one roof 🙂

@sghoweri
Copy link

@sghoweri sghoweri commented Jan 12, 2018

^ but yes @bradfrost you nailed it. We already have a working prototype (in the form of a tiny NodeJS file + PHP server / service running in the background) that takes in a reference to a PL Pattern + local data, sends that off to the Pattern Lab PHP / Twig engine for rendering, and returns back the rendered HTML.

Still some work to do (like getting @aleksip's refactor work folded in) but definitely a solid indication that we can keep running in this direction

@raphaelokon
Copy link

@raphaelokon raphaelokon commented Jan 13, 2018

Big vote from me too @bmuenzenmeyer

Monorepo architecture for Core + Engines
I'd add the CLI under the same monorepo umbrella

@bradfrost
#37 (comment) => 👍

Offtopic: I had the idea to implement the Pattern Lab core in a lower-level language like C++ or Rust and maintain it as a lib with bindings to Node, PHP etc.—similar to libsass approach. This would make the core portable (albeit with the downsides to compile to diff archs obv) and a single source lib. I haven't made any assessment if this might be viable but thought to just put out the idea to falsify it.

@sghoweri
Copy link

@sghoweri sghoweri commented Jan 13, 2018

If Pattern Lab had a proper spec, detailing everything at the required level, and an acid test project covering the entire spec, I don't think there would be a need to declare an authoritative representation.

Do we see value in having a proper spec? Is it worth the extra work? Personally I think it depends on whether it is possible to have true Twig support for the Node version.

Let's assume for just a second that we don't have a legit, testable spec that'd basically gives you the "🌟 Works With Pattern Lab" seal of approval:

  1. How do we ensure that all the different Pattern Engines out there (plus the new ones that pop up) don't break or don't fall behind with ongoing updates? Is the idea here that all PL engines (mustache, Twig, React, etc) live under the same monorepo?

  2. The even bigger question for me: how can we continue to improve upon the Frontend Architecture of Pattern Lab (ie. Styleguidekit Asssts Default) and refactor the parts that are badly in need of a rewrite without a spec?

I totally 100% get that having a maintained, up to date spec / API is a ton of work, however without having some two-way contract everybody can work towards, how do we further widen the doors on improving the PL ecosystem besides "if you don't like the styleguidekit PL ships with, sorry!"?

CC @aleksip @bmuenzenmeyer @EvanLovely @bradfrost

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 13, 2018

How do we ensure that all the different Pattern Engines out there (plus the new ones that pop up) don't break or don't fall behind with ongoing updates? Is the idea here that all PL engines (mustache, Twig, React, etc) live under the same monorepo?

Today we do this by shipping each node PatternEngine as a devDependency of core, and place all tests within there. They run as part of our Travis CI. They mostly cover the unique elements of a language, such as partial inclusion, rendering, etc. The monorepo should make these even easier.

The even bigger question for me: how can we continue to improve upon the Frontend Architecture of Pattern Lab (ie. Styleguidekit Asssts Default) and refactor the parts that are badly in need of a rewrite without a spec?

I have a plan for how to do this - which I will outline as part of pattern-lab/styleguidekit-assets-default#104. I think it hinges on documenting what core creates post-build in the form of data to output. Today that is poorly documented and works by virtue of convention with parts of styleguidekit-assets-default and styleguidekit-mustache-default - I know how we can do better.

Offtopic: I had the idea to implement the Pattern Lab core in a lower-level language like C++ or Rust and maintain it as a lib with bindings to Node, PHP etc.—similar to libsass approach. This would make the core portable (albeit with the downsides to compile to diff archs obv) and a single source lib. I haven't made any assessment if this might be viable but thought to just put out the idea to falsify it.

This sounds interesting but I'll admit it's not really on my radar or area of interest.

@james-nash
Copy link

@james-nash james-nash commented Jan 15, 2018

Sounds like a good plan to me.

@EvanLovely
Copy link
Member

@EvanLovely EvanLovely commented Jan 18, 2018

OK, as the maintainer of Pattern Lab PHP Core and someone who’s built a career around the integration of Design Systems with CMSs like Drupal, I’ve got a lot of opinions on this - I take this all working correctly very seriously.

If one is to build an in-depth user interface in an environment that it ultimately will not reside in, there is an understandable amount of fear and hesitation. Why should I build this in Pattern Lab if it won’t work in the CMS? That is a question that can be only put to rest with a confident answer that is basically along the lines of “because it’s compiled the same way”.

When I brought this idea up to Brian & Brad, I phrased it something like this:

Let’s ask a question: What does Pattern Lab do?

  1. After grabbing some configuration, it lifts some global data.
  2. Then it cruises through a patterns folder and for each pattern it:
    1. Merges data specific to that pattern with the global data if it’s there
    2. Renders the template: Passes that data to the pattern template, then receives back HTML
    3. Looks to see if there are any variation datas to render (ie button~primary.json) and does the above
  3. After getting HTML back, it places it in ./public
  4. Creates a UI that let’s you browse those HTML files: this is the menu, the demos of each HTML file, and some other niceties like docs, code samples, and some more stuff that I’m glossing over.

With the exception of the “Render the template” step, the PHP & JS implementations of Pattern Lab Core are redundant. And challenging to maintain. And other bad things associated with not being DRY; it’s WET: Write Everything Twice.

However, it’s extremely important for that “Render the Template” step to happen correctly. We need to ensure that creating a deliverable in Pattern Lab is not a more evolved static mockup that solely exists to inform the ultimate implementation and is eventually thrown away, but it is something that is utilized, propagated, and pointed to as the source of truth. The only way to do that is the confidence in the parity between Pattern Lab and the eventual production environment. There’s a lot to that whole “parity” bit, but for this conversation it’s mainly about consistent template rendering pipelines.

I’ve done a lot of experiments, and I can tell you: Twig in JS & PHP are not similar enough for me to sleep quietly at night while I build out templates that will go down both rendering pipelines.

So I had an idea: what if PL JS Core could just ask PHP, “Here’s some data, pass it into that template, give me back HTML, and I got the rest”. Cause, you know: that’s how code languages actually talk to one another.... :) It wouldn’t be that hard, right? Run initialization where you setup global goodies, then for each template: here’s JSON data, and a path string to the template, please return a string of HTML. There’s several ways to solve that (CLI & API come to mind) and I know: devil in the details.

The prospect of not having to keep feature parity between the JS & PHP cores of Pattern Lab is compelling enough to truly pursue this as the future of what the core does: aggregating data, giving it to templates, collecting compiled HTML in a directory and providing an elegant UI for browsing it. Why should we write that logic in PHP & JS and attempt to keep it in sync? It’s really the template rendering we want unique.

Now: back to reality. This is open source development. Are we deprecating Pattern Lab PHP Core due to the possibility that we can consolidate redundant logic while preserving existing functionality? No. Is that the future? I really hope so. Will a future solution that utilizes a JS core and a PHP Twig Template render be tested to determine that it does everything that the full PHP stack did beforehand? Hell ya! Could we use some help doing it? Please, don’t knock over each other lining up :) If we succeed and it all feels good, will we deprecate Pattern Lab PHP Core? With a small tear in our eye and a shot 🥃 taken for Dave Olsen, yes.

A few final thoughts:

  • What I would love to see from any who want to contribute is a package that works with the new PL JS Core 3 beta that can render using PHP Twig
  • I love & encourage monorepos tons; do it for PL! It’s great when you need to work on and coordinate releases across several packages. Use Lerna and talk to @sghoweri. However, can we please spin up another thread for that convo? This one’s heavy already.
  • This is all going to take longer than we think, I’d love to get PL PHP Core in a more maintainable state. This could be help & improvements to the current 2.x or it could be a clean implementation for 3.x using modern tools; something @aleksip & I have talked about and sounds awesome.

You all rock!

@sghoweri
Copy link

@sghoweri sghoweri commented Jan 18, 2018

What I would love to see from any who want to contribute is a package that works with the new PL JS Core 3 beta that can render using PHP Twig

@EvanLovely @aleksip I think we've got a pretty strong lead with this via the Pattern Lab PHP Server POC I was messing with a couple months ago. At a minimum, this should help us figure out what other work is required + how much PHP code we can drop / refactor into Symphony before things no longer work

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 18, 2018

Well put @EvanLovely - thanks for your thoughts. Frankly I don't think any of this could be done without your support.

With the exception of the “Render the template” step, the PHP & JS implementations of Pattern Lab Core are redundant. And challenging to maintain. And other bad things associated with not being DRY; it’s WET: Write Everything Twice.

Exactly 👍

However, it’s extremely important for that “Render the Template” step to happen correctly. We need to ensure that creating a deliverable in Pattern Lab is not a more evolved static mockup that solely exists to inform the ultimate implementation and is eventually thrown away, but it is something that is utilized, propagated, and pointed to as the source of truth.

Without a doubt this is the goal. This is the exact plan for patternengine-node-react - patterns / components authored within Pattern Lab can be directly consumed in a production environment. I think we are all after the same thing.

So I had an idea: what if PL JS Core could just ask PHP, “Here’s some data, pass it into that template, give me back HTML, and I got the rest”. Cause, you know: that’s how code languages actually talk to one another.... :) It wouldn’t be that hard, right? Run initialization where you setup global goodies, then for each template: here’s JSON data, and a path string to the template, please return a string of HTML. There’s several ways to solve that (CLI & API come to mind) and I know: devil in the details.

The separation of concerns within Pattern Lab Node already enables this capability. All rendering is deferred to a particular engine's implementation of data + template = HTML. Pattern Lab Node 3.0.0-Alpha.6 was the first release to support async rendering, truly enabling PatternEngines to do whatever they want, for however long it takes, to deliver that HTML to core. The current experimental patternengine-node-twig (here) is out of date in regards to async, but the sky is the limit with what you want to write in here. This flexibility is what already enables Mustache, Handlebars, Underscore, Nunjucks, React, Liquid, and Thymol engines to operate as essentially plugins. I wholeheartedly agree that we need to nail Twig support.

The prospect of not having to keep feature parity between the JS & PHP cores of Pattern Lab is compelling enough to truly pursue this as the future of what the core does: aggregating data, giving it to templates, collecting compiled HTML in a directory and providing an elegant UI for browsing it. Why should we write that logic in PHP & JS and attempt to keep it in sync? It’s really the template rendering we want unique.

and

Now: back to reality. This is open source development. Are we deprecating Pattern Lab PHP Core due to the possibility that we can consolidate redundant logic while preserving existing functionality? No. Is that the future? I really hope so. Will a future solution that utilizes a JS core and a PHP Twig Template render be tested to determine that it does everything that the full PHP stack did beforehand? Hell ya! Could we use some help doing it? Please, don’t knock over each other lining up :) If we succeed and it all feels good, will we deprecate Pattern Lab PHP Core? With a small tear in our eye and a shot 🥃 taken for Dave Olsen, yes.

Awesome. This issue was never meant to change anything overnight. My goal was to communicate our long-term intent, in writing, rather than wishful thinking and what-if conversation.

What I would love to see from any who want to contribute is a package that works with the new PL JS Core 3 beta that can render using PHP Twig

Anyone that wants to do this can have carte-blanche access to https://github.com/pattern-lab/patternengine-node-twig. There are some small things I could do to prep it, but it's largely ready to start playing with against PL Node 3.0.0-Alpha. I think we started some initial testing within pattern-lab/patternlab-node#285 and pattern-lab/patternlab-node#554 but we could most certainly do much more. @geoffp is building React support in an awesomely TDD style. @sghoweri @aleksip you are welcome to take it over - I don't intend to focus on this in the near-term.

I love & encourage monorepos tons; do it for PL! It’s great when you need to work on and coordinate releases across several packages. Use Lerna and talk to @sghoweri. However, can we please spin up another thread for that convo? This one’s heavy already.

This is in the works. I think my plan is to pull the trigger on this in February, as part of a hack day that @geoffp and I are both attending. I'd be happy if we had PL Node Core and all it's engines under one roof during that time. Perhaps the UI repos too. I can see the UI work in regards to workshop vs storefront benefiting from monorepo in addition to front end re-architecture in addition to core refactoring. It's a lot of moving parts - but definitely attainable. @bradfrost what are your expectations about when you'd hoped to get PL baked in with workshop vs storefront rendering?

This is all going to take longer than we think, I’d love to get PL PHP Core in a more maintainable state. This could be help & improvements to the current 2.x or it could be a clean implementation for 3.x using modern tools; something @aleksip & I have talked about and sounds awesome.

I agree - this was never an "okay, let's shut down PHP and delete the repo" situation. Communicating intent to the community helps frame conversations and focus our goals. We've all long-since acknowledged the redundancy of our platforms - this marks the beginning of a road to unification.

@geoffp
Copy link

@geoffp geoffp commented Jan 18, 2018

As @bmuenzenmeyer says, the async render work should allow us to delegate really anything we want to anything we want. As much as I'd like to see the JS Twig implementation get up to snuff, in a sense, it's also redundant with the PHP version, and in any case may be perpetually chasing it for freshness and correctness. So I'd love to see a POC Twig engine that delegates intelligently to PHP for perfect fidelity. So I'm rooting for @sghoweri and @EvanLovely.

I could see this approach getting complicated for templates that call each other, but it seems like the potential challenges with that are eased by the fact that we have a badass pattern graph now (shoutout to @tburny). If we have to render a PHP-based template that results in a tree of ten nested template calls, since we already know what that whole tree looks like, we can pass the whole flattened set to PHP/Twig along with data and get a coherent response. All the merging of JSON data can happen on the JS side.

+1 for monorepo, also -- I think that will dramatically ease and accelerate the evolution of all related packages, especially engines and the UI.

As I work on the React engine, having put in about a year working on a team doing UI development with Storybook/React, I'm focused on making PL into the first-class component development environment we wished we had had available to us back when we migrated to Storybook along with React adoption. The use case that I'm passionate about is this: PL should be able to produce well-organized, well-documented, portable, publishable libraries of consumable UI code. For creating design systems in particular.

@bradfrost
Copy link
Member

@bradfrost bradfrost commented Jan 18, 2018

As I work on the React engine, having put in about a year working on a team doing UI development with Storybook/React, I'm focused on making PL into the first-class component development environment we wished we had had available to us back when we migrated to Storybook along with React adoption. The use case that I'm passionate about is this: PL should be able to produce well-organized, well-documented, portable, publishable libraries of consumable UI code. For creating design systems in particular.

I love you, @geoffp.

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 19, 2018

First of all, and most importantly, I want to say that I'd really like this to happen, and hope to be able to help make it happen, and to have a fully working PHP Twig PatternEngine for Pattern Lab Node. Still, a few challenges and differences from current functionality come to mind.

As @geoffp pointed out, templates calling other templates could get tricky. If Pattern Lab does things in JavaScript (like flattening template sets and merging data) that in the production environment are done in PHP Twig, I'm not sure if we can guarantee parity in all situations?

There is a small but important difference in how (at least the PHP version of) Pattern Lab actually works in step 2: data for all patterns is gathered and processed before rendering, and all Pattern Lab data is available for plugins (and consequently for Twig) when a pattern is rendered. This is a very powerful feature, which would disappear if only template specific data is passed to PHP Twig. But maybe it is an acceptable regression?

The refactoring work I've been doing on the internals of PL PHP Core is really big in scope: it touches almost every class in the current version. If we are going 100 % for the unified PL Node Core, I don't think we should use our limited resources on PL PHP, other than for maintaining the current version. Backwards compatibility issues and possible new bugs/issues introduced by the refactoring would just create more work on the PHP version.

I see the native PHP part of PL Node's Twig PatternEngine as something completely new and separate from PL PHP, so the Core refactoring I've been doing (Symfony Console/Config/Dependency Injection/Stopwatch etc.) would most likely not affect it in any way. I haven't changed the way PatternEngines are added and used.

I guess this is obvious, but the new native PHP part should support an easy way to extend Twig functionality, and ideally provide BC with the extension mechanisms in the current PHP Twig PatternEngine and PL PHP Core.

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 22, 2018

@aleksip

There is a small but important difference in how (at least the PHP version of) Pattern Lab actually works in step 2: data for all patterns is gathered and processed before rendering, and all Pattern Lab data is available for plugins (and consequently for Twig) when a pattern is rendered. This is a very powerful feature, which would disappear if only template specific data is passed to PHP Twig. But maybe it is an acceptable regression?

I'd like to understand more about what you mean here, and clarify a few points. In Pattern Lab Node all of the following are true:

  1. All gathering occurs before a single render takes place
  2. A plugin system exists, with the entire Pattern Lab object at time of invocation being passed to each event at broadcast time
  3. Global data, global listItems, (some day catagory-level data), pattern-specific data, pattern-specific listItems, pattern parameters, and all pattern links are passed into a pattern's template as data when rendering
  4. If Pattern B includes Pattern A, Pattern A's .json data (if present) is not included in Pattern B's rendering. Data inheritance to the contrary is considered not on-spec, though the topic comes up occasionally.

I see the native PHP part of PL Node's Twig PatternEngine as something completely new and separate from PL PHP, so the Core refactoring I've been doing (Symfony Console/Config/Dependency Injection/Stopwatch etc.) would most likely not affect it in any way. I haven't changed the way PatternEngines are added and used.

Sure. Whatever you guys have in flight within PL PHP Core should continue - acknowledging that it will be perpetuating the duality of our solutions. Building the Twig PatternEngine within PL Node is probably a smaller task 😄 - one that already has much of the boilerplate complete but the innards yet to be baked.


At this point I feel like we are all aligned. The intent of this issue is not to stifle or shut down any in-progress PL PHP work. The intent of this issue is to communicate in writing our long-term goal, as we have over a couple voice calls, to unify the core Pattern Lab experience under the Node project.

Vote passes.

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 22, 2018

@bmuenzenmeyer PL PHP works pretty much just like PL Node then!

Currently it is possible for PL PHP plugins to have control over the rendering by extending the template engine, and for these extensions to have access to all PL data, regardless of what data is passed in the actual render call by PL. This is probably true for PL Node too.

The PHP part of PL Node's Twig PatternEngine will surely provide a way to extend Twig, but it is the full access to all PL data we might be losing, unless we figure out some nice way to provide it.

@bmuenzenmeyer
Copy link
Member Author

@bmuenzenmeyer bmuenzenmeyer commented Jan 22, 2018

The PHP part of PL Node's Twig PatternEngine will surely provide a way to extend Twig, but it is the full access to all PL data we might be losing, unless we figure out some nice way to provide it.

I understand better now. The render api for each PatternEngine can easily accommodate an additional parameter that is the whole data structure - I think Evan has alluded to as much. I think this effort will be greatly improved with the monorepo - exciting times ahead. Thanks for your thoughts.

@EvanLovely
Copy link
Member

@EvanLovely EvanLovely commented Jan 25, 2018

For the record; my vote: 👍🏼

@EvanLovely
Copy link
Member

@EvanLovely EvanLovely commented Jan 25, 2018

Check it out; here's a basic way to isolate the Twig PHP engine: https://gist.github.com/EvanLovely/165329dc361ff07b669e3ca365ec5ffe

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 25, 2018

One concern I have about a PHP Twig PatternEngine for Pattern Lab Node is how much it would affect performance. Creating a new PHP process, setting up the Twig environment, encoding/decoding JSON and passing pattern/all data for each pattern render call sounds very resource intensive.

Pattern Lab PHP Core renders patterns in one isolated stage, in PatternCodeHelper::run(), which is basically just a foreach loop with render() calls to the PatternEngine. If the Node version works in the same way (or can be refactored to work in the same way), we could just move this loop to the PatternEngine, and have all patterns rendered in one call passing all data. This would also mean that there would be no need to flatten template sets and merge data on the JS side. It would also probably be possible to use the same Twig PatternEngine for both PL PHP and Node, with the Node PatternEngine being just a thin wrapper around the PHP PatternEngine.

@EvanLovely
Copy link
Member

@EvanLovely EvanLovely commented Jan 25, 2018

I completely agree on the performance hit of spinning up a new Twig Environment for each template. That was just a simple proof of concept.

Also, know that if the template sets do get flattened, then I view that as a non-starter.

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 26, 2018

It would also probably be possible to use the same Twig PatternEngine for both PL PHP and Node, with the Node PatternEngine being just a thin wrapper around the PHP PatternEngine.

The data stores in the Node and PHP versions would have to be identical/compatible for this to work. Official JSON schemas for all data stores would in any case be useful for implementing plugins (and PatternEngines, if the idea of moving the render loop from Core to PatternEngines is considered a good one).

@aleksip
Copy link
Member

@aleksip aleksip commented Jan 26, 2018

We should open a specific issue for implementing PHP Twig PatternEngine for Pattern Lab Node. Should it be at pattern-lab/patternengine-node-twig, or should we start a new repo? Do we still want to officially support twig.js?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.