PatternEngines ongoing work, round 3 #199
Conversation
Remove extra/unnecessary word
Dev push for v0.15.0
…ering style modifiers
to mustache! Hooray!
factory in the pseudopattern hunter; misc. code cleanup; hide some debug console.logs behind the flag
utilities module; move isPatternFile into pattern_engines module since that's where the knowledge it needs is concentrated
verbose syntax, style modifier syntax, pattern parameter syntax, and single and double quoted strings in pattern parameters with escaped quotes. Making this hurt my brain very badly. Update the appropriate unit test with all the test cases used to develop the regex, plus some flown in from other unit tests.
|
Greetings from a fascinated onlooker! I just want to say thank you to you and certainly everyone else (especially @bmuenzenmeyer) for rocking and rolling on the node version of Pattern Lab. |
|
Hi Brad! Thanks right back at both of you for Pattern Lab/Node and Atomic Design. They've been crucial to our ongoing work on the responsive m.target.com. And @bmuenzenmeyer has made it a real pleasure to contribute and be involved. |
|
@bradfrost Hey there. Thanks for the kind words - it's fuel to make the port more than I'd ever imagined on the outset. You and @dmolsen and everyone interested has made it what it is today. @geoffp SUPER DUPER DUPER interested in knowing more about m.target.com and how you use the project. I'd be humbled to add a wiki page or prominent shoutout to project that use patternlab. Perhaps this is something that patternlab.io could host Brad? I'm getting a bit buried on this pattern-engine work and other pattern lab documentation/issues coming in but I will try to review more in-depth soon. Thanks |
the verbose partial syntax without the .mustache file extension, as per http://patternlab.io/docs/pattern-including.html
|
@bmuenzenmeyer, I know, I'm really sorry about the sheer volume of the pattern-engine code. I know it's hard to review; I have a window of opportunity during which I can contribute this work, so I'm trying to get it as close to "done" as I can while the window's still open. I'd love to tell you guys all about how we use it, and why we chose the Node port. I feel like I should find someone here who can confirm for me what it's okay for me to talk about before I start yapping all over the public internets. It's a big company. |
|
I'd be over the moon crazy excited to hear about it (publicly or On Wed, Dec 2, 2015 at 10:54 AM Geoff Pursell [email protected]
|
|
I got a green light! 🚦 I don't speak for Target as a whole, of course, just for myself and my team. We use Pattern Lab/Node as the primary front-end development environment for most of the chunks of UI, large and small, on the new m.target.com, which is a fully responsive new version of Target's main e-commerce platform. Right now it's targeted at phones and tablets. Atomic design has been our chosen methodology from the beginning of this phase of development (about a year ago). We've talked to Brad and Josh Clark, and tried to get our design taxonomy right form the beginning, which is incredibly hard to do when you're trying to go really fast. We're still grappling with how to classify some things. Back then, we evaluated PL/PHP and PL/Node; we chose Node for two reasons: one, we're moving toward Node.js on the back end, and have no preexisting PHP usage, so it was a better fit for our developers. But more importantly: we eventually want to operationalize our templates, and because all our work is in the JS domain (mostly in the browser), we need support for JS templating engines. Back then, we thought we were moving towards Handlebars, so we forked PL/Node and hacked it up (hastily) to replace Mustache with Handlebars. That was pretty easy, since Handlebars is a superset of Mustache, but there were two problems with it: one, it was still hard-coded to one template engine, and two, we wound up not using that engine. So now, we are doing it (more) right, making it work flexibly, and giving back to the community, which it is an honor and a privilege to do. We need to make it support Handlebars (to support our existing library of patterns), but also Underscore (our on-the-ground reality of the moment) and eventually whatever "real" web component system we wind up with, whether it's Polymer/Web Components or React. We love developing UI without the overhead of a full web app, a11y loves using patterns as a11y "unit tests", and we love being forced to think about UI structure through Atomic. |
I haven't run this code yet, but the this PR addressed this last time. Perhaps it's a simple order of operations bug again. I want to respond more long form to your latest comment. Thanks for writing! |
true for a pseudopattern JSON file and it wasn't. In fact, its unit test was lying to us. Lies! Also: implement a proper isPseudoPatternJSON() function in pattern_engines to centralize that logic and write a unit test for it; move isPatternFile unit test to the right file
|
@geoffp per Salem, when you are ready I will merge this. |
|
@sghoweri, that's fine by me. Alternatively, you could submit a pull request against my fork, and then I could do the merge there right away. I think I still have to get my branch up to date with the latest stable release, so I may be in merge-land anyway. @bmuenzenmeyer fixed the out-of-order thing. It was totally my fault -- the isPatternFile() function should have returned true for a pseudopattern JSON file name and it wasn't, so processPatternIterative() wasn't registering the preliminary pseudopattern at the right time. It was getting registered later in the process, though, hence the out-of-order menu. I even had the unit test written to lie to us and say everything was fine. |
|
@bmuenzenmeyer cool. I want to take one more look at the unit test that's still failing and see if I can get us into a completely clean state, if that's okay with you. |
…ern-engines Conflicts: builder/lineage_hunter.js builder/list_item_hunter.js builder/object_factory.js builder/parameter_hunter.js builder/pattern_assembler.js builder/patternlab.js builder/pseudopattern_hunter.js test/pattern_assembler_tests.js
repeated renders' test to actually, you know, test verbose partials
|
Okay! I'm ready to let go for the weekend. Hopefully this is a clean platform on which @sghoweri can rock and roll. We're passing all the unit tests, and up to date with the latest release! Exactly the kind of Friday afternoon situation you want. @bmuenzenmeyer, this includes the tricky 0.15.1 merge (fingers crossed) and a non-liar unit test for the list item hunter/verbose partial syntax. There was also one more Mustache-specific regex in the pattern assembler (getPartialKey) that's been factored out. |
|
Great work! I am going to accept this so Salem can start looking at it easier perhaps. Will try to review this weekend too amidst trying to finish my own v1.0.0 work |
PatternEngines ongoing work, round 3
|
+1 Awesome - thanks guys! |
|
OK - just a really quick update before I gotta hit the hay. Today I successfully managed to get my ongoing Twig template support updates working with the latest pattern-engines branch updates from the weekend. There's a few things I'll want to run by you guys before my next PR goes out but so far so good. Even better news: I managed to get the lineage mapping (and code viewer links for these) working in addition to pattern modifiers workings with the Twig engine as well. Haven't had a chance to take a crack at some of the other regex matches (like style modifiers) yet but for all intents and purposes these Twig updates are pretty damn close to the feature parity of the mustache templates (plus all the goodies Twig provides us). I'll see if I can get these updates checked in tomorrow or Wednesday so we can keep the ball rolling on this! |
|
Awesome @sghoweri looking forward to it! It's amazing to see this come together, at a time when I finally finish up some of the last v1.0.0 work too, so we can move into the future with these engines 🚀 |
|
Hey all - I just pushed some small changes to |
|
Should I keep my branch up to date with |
|
It needs to get merges sooner or later, either by you or I. I suggested it only because I know you were in the unit tests a bit and they changed a bit, along with cleaning up a bug in pseudopattern generation. You can see the commit here. The only remaining item on my upstream end is this one which I am working on now - then I'll be focusing 100% on engines and post 1.0.0 world. |
|
Good call. Seems pretty easy to merge. @sghoweri, I super-want your latest work because I'm going to start on the Handlebars engine and I bet real money you've already fixed a bunch of the problems I'm going to find. |
|
This is fascinating to watch from the outside. Thanks for collaborating on this. |
|
oh, and @geoffp, thanks for sharing your notes on your experience. very cool to hear that the concept is being useful for folks. |
|
@geoffp Have the code up on my pattern-engines fork if you wanted to take a look (https://github.com/sghoweri/patternlab-node/tree/pattern-engines), albeit not 100% done yet. Besides to make the code viewer more template-agnostic, I'd imagine you'd be most interested in changes I had to make to the pattern_assembler.js (so non .mustache templates are picked up) + the overall process it took to get Twig working in the pattern_engines folder since I'd imagine rolling in other templating languages (like Handlebars) would be very similar, if not a bit easier. Still cleaning up some stuff but lemme know if you had any questions in the meantime! |
PatternEngines ongoing work, round 3
PatternEngines ongoing work, round 3
PatternEngines ongoing work, round 3
PatternEngines ongoing work, round 3
PatternEngines ongoing work, round 3
PatternEngines ongoing work, round 3
The day's work.
TODO:
Investigate why pseudo-patterns are being placed in the menus out of orderFinish factoring out all direct uses of mustacheHunt down all Mustache-specific "finder" regexes in the hunters and move them into the Mustache pluginfinders in pattern_assemblerUse or remove the util moduleMigrate unit tests to use "real" instantiated oPatternsMachinery to create empty/custom oPatterns programaticallyReplace object-literal fake oPatterns in unit tests with real oPatterns