Basics of being dependent and passing in config

[EvanLovely]

OK, so here's the absolute basics of being able to var pl = require('patternlab-node')(config);. The big problem that remains is that most of the stuff that is ran from builder/patternlab.js (the require-ed file) is running from the working directory of node_modules/patternlab-node/, so this PR doesn't complete #221 but simply moves towards it and allows it to work if you know what you're doing. The way I'm getting around this (for now) is with this:

var config = fs.readFileSync('./config--pattern-lab.json');
var pl = require('patternlab-node')(config);
// get directory
var plnDir = path.dirname(require.resolve('patternlab-node/package.json'));
// save place
var originCwd = process.cwd();
// change to pl directory
process.chdir(plnDir);
// build it
pl.build(true);
// come back
process.chdir(originCwd);

This PR also creates the distinction between dependencies and devDependencies – if this is a required dependency in other projects, only the dependencies get installed; otherwise when npm install is ran here, both get installed.

I expect this PR to be easy to merge and should not disrupt other workflows or uses and hopefully we can build upon this approach by making builder/patternlab.js run in the correct working directory (I believe it's a distinction between process.cwd() and __dirname, but I'd need to look closer to really know).

🍻

[bmuenzenmeyer]

@EvanLovely you are an open source juggernaut! Thanks for this work - I am feeling a bit overwhelmed by the support and interest of late, so I might be slow on reviewing everything. Please put up with me ⏳

[EvanLovely]

Thanks! No worries. Hopefully I can help more soon as we use PatternLab pretty heavily at work.

[bmuenzenmeyer]

Hi @EvanLovely forgive my ignorance on this following issue...

If this is accepted, creating the distinction between devDependencies and dependencies, won't a user pulling down patternlab node via npm install patternlab-node and running "vanilla" only receive the dependencies? I feel this could be detrimental without proper documentation, as people simply running npm install patternlab-node may not realize they will not get the entire harness.... whereas users intending to use patternlab node as a dependency will definitely have upstream task harnesses/etc.

... thinking and googling ensues ...

This is just now starting to click more for me! Ok. So really the "vanilla" documentation should be moe clear (this is more for my benefit than yours, FYI don't be offended 😊 ):

---

Download

You may get Pattern Lab a couple ways, depending on your intended use:

Stand-Alone Installation

• Download the latest release of patternlab-node from Github
• or clone the git repository https://github.com/pattern-lab/patternlab-node.git
• From the command line at your extracted file location, run npm install
• NOTE Doing this will install grunt (or gulp if you follow the instructions later in this README) along with the helper libraries necessary to shuffle assets and serve the Pattern Lab website / styleguide.

Pattern Lab as a Dependency

• run npm install patternlab-node
• NOTE Doing this will omit grunt, gulp and their respective helper libraries, assuming you are handling a lot of asset shuffling upstream and only using the ./builder/patternlab.js API.

---

Please let me know if I have this right. If I do, I'll accept this and get the README updated to reflect the above after some testing.

cc @geoffp this might be of interest to you too.

[EvanLovely]

I think that putting out documentation on how to use this as a dependency is too early. Due to how the paths are set up in builder/patternlab.js one needs to change the working directory to node_modules/patternlab-node/ for the builder to work (even with custom paths). This more or less lays the ground work for this repo to be used as a dependency.

I also think that a Yeoman Generator would be the way to go for scaffolding out a "Stand-Alone Installation" – we could even ask questions like if they want Sass or not, Grunt or Gulp, etc and then create a different file structure based on those questions.

Also, a good tidbit you may find useful for testing is the ability to supply different arguments to npm install; so if you wanted to see what a npm install patternlab-node would be like after this PR merges, you can run:

npm install git@github.com:EvanLovely/patternlab-node.git#feature/enable-as-dependency

or a shorthand for GitHub URLs:

npm install EvanLovely/patternlab-node#feature/enable-as-dependency

Doing so and comparing to npm install patternlab-node I can see my version installs the stuff in dependencies into node_modules/patternlab-node/node_modules/ whereas the regular installs nothing into it (b/c all of them are devDependencies). In either case if npm install is ran in the patternlab-node/ directory the same set of dependencies are installed: all of them.

So basically what I'm saying is that I don't think anything needs to get added to the README: all dependencies will get installed with a npm install as documented after a download and the process to run this repo as a dependency is too cumbersome for mass consumption due to working directory challenges; however this will set the foundation for that work to be done – work I look forward to helping with.

[bmuenzenmeyer]

I think that putting out documentation on how to use this as a dependency is too early. Due to how the paths are set up in builder/patternlab.js one needs to change the working directory to node_modules/patternlab-node/

Okay I finally get this. Your example code was useful, it wasn't clicking the first time through my head. Very happy to have you on board to help with thinking through this problem!

I also think that a Yeoman Generator would be the way to go for scaffolding out a "Stand-Alone Installation" – we could even ask questions like if they want Sass or not, Grunt or Gulp, etc and then create a different file structure based on those questions.

I like the idea in general, but at least at this time I want to keep Yeoman outside the scope of this repository. I think that could change as it matures more (or perhaps others focus on this), but right now my focus is this dependency work, pattern engines, and making things more modular/extensible...I believe others have already wrapped the project and are essentially doing this work, but I don't recall who or where at this time. I've wondered if my perspective toward Yeoman is skewed because I don't go through the motions/pain of initializing projects with the same frequency as those in client services.

Thanks for the tips on testing. I've tested PRs locally for a while, but never tried npm installs - so that's pretty cool. Now that I understand the state of this issue more, I'll try to get it merged post-haste.

[geoffp]

It seems like the struggle here is against direct references to config.json. Here's my quick search:

builder/lineage_hunter.js:20:29:            var config = require('../config.json');
builder/list_item_hunter.js:20:26:          config = require('../config.json'),
builder/object_factory.js:17:35:            var config = fs.readJSONSync('./config.json');
builder/pattern_assembler.js:21:37:         var config = fs.readJSONSync('./config.json');
builder/patternlab.js:27:42:                patternlab.config = fs.readJSONSync('./config.json');
test/engine_handlebars_tests.js:23:27:      config: require('../config.json'),
test/pattern_assembler_tests.js:290:43:     patternlab.config = fs.readJSONSync('./config.json');
test/pattern_assembler_tests.js:615:43:     patternlab.config = fs.readJSONSync('./config.json');
test/pattern_assembler_tests.js:707:43:     patternlab.config = fs.readJSONSync('./config.json');

The first one is completely unnecessary, the next two just need the debug flag, but don't have access to the main patternlab object, which is where the config data lives. The fourth I think is mixed, but the problem areas are just debug flag. The rest are unit tests that may not ever have access to the real patternlab object -- that's a challenge.

Maybe we can cheat and create a global variable (not even sure how that works in Node, honestly) for the debug flag. And maybe we can just say that the unit tests only work in a standalone PL -- I personally only run them in a standalone copy.

[geoffp]

Also, I agree with @bmuenzenmeyer on Yeoman -- it's a fine tool where it works, and I'm not against code generation or scaffolding in general. But I think we're trying to aggressively reduce the number of moving parts and external dependencies needed to get up and running with PL to tear down barriers to use.

[bmuenzenmeyer]

Maybe we can cheat and create a global variable (not even sure how that works in Node, honestly) for the debug flag

I've started reading more of the node documentation to determine how best to do this, but yet I think it's possible to place this flag somewhere more intelligently.

https://nodejs.org/api/modules.html
https://nodejs.org/api/globals.html

[geoffp]

It might be nice to be able to say something like:

grunt serve --debug
gulp serve --debug

rather than editing the config.

[bmuenzenmeyer]

Agreed
On Thu, Jan 14, 2016 at 9:43 AM Geoff Pursell notifications@github.com
wrote:

It might be nice to be able to say something like:

grunt serve --debug
gulp serve --debug

rather than editing the config.

—
Reply to this email directly or view it on GitHub
#223 (comment)
.

[EvanLovely]

OK, lots to respond to here, so I'm going to do this in parts; first off @geoffp I'm not seeing those hard coded references to config.json you mention 🤔 – are you on dev? I checked in master too.

Here's the first few you mentioned and where I'm looking:

• https://github.com/pattern-lab/patternlab-node/blob/dev/builder/lineage_hunter.js#L20
• https://github.com/pattern-lab/patternlab-node/blob/dev/builder/list_item_hunter.js#L20
• https://github.com/pattern-lab/patternlab-node/blob/dev/builder/builder/object_factory.js#L17

The only two references I can find to config.json are test/pattern_assembler_tests.js#L212 (which I think is fine to leave) and the change I'm proposing in builder/patternlab.js.

[EvanLovely]

Alright, next up: I've just added a new commit to this PR. When references to config.json are resolved using, the default is to resolve from where the command was ran from, the current working directory, which this project has made the understandable assumption will be the root of this repo. However, if it's ran from somewhere else, say two directories up, then references to config.json are resolved from there.

This PR enables people to pass in a config object to use instead of config.json, but we still would like it to get read as a default, so my last commit utilizes the __dirname variable (The name of the directory that the currently executing script resides in.) and a path.resolve() to get to the config: basically saying: start from directory where this file resides, and go find ../config.json – instead of: find config.json in this current working directory.

-  patternlab.config = config || fs.readJSONSync('./config.json');
+  patternlab.config = config || fs.readJSONSync(path.resolve(__dirname, '../config.json'));

I believe this pattern could be applied to other places and that would solve having change the working directory to the root of this repo before executing.

[geoffp]

@EvanLovely you're right, I think many of those are probably just in the pattern-engines branch, which is where I'm working at the moment. Can you address the ones in the pattern_assembler tests?

This new approach makes me way less nervous than changing the working directory.

[EvanLovely]

Good to hear! I think the reference to ./config.json in test/pattern_assembler_tests.js#L212 is fine to leave as that's a test and those will definitely be ran from the root of this repo and not from a parent project.

Agreed on not like the approach on changing the working directory – that's why I suggested not documenting that method for others to follow.

[bmuenzenmeyer]

I checked this out and merged after a quick test.

@geoffp and @EvanLovely - from your perspectives, what's left to address on this PR or #220?

I am aware of #229 and will be taking that one

[geoffp]

It would be nice but not necessary to have a global variable for the debug flag.

I'd like to try replacing my copy-the-config solution with this one today, if I can get to it, since I have a real-world NPMified configuration up and running. That should give us one more vote of confidence.

[geoffp]

It works for me! My gulpfile is much simpler without the fussy config file copy step, and everything seems to work. I guess all we need now is some documentation.

@bmuenzenmeyer, feel free to assign me any work you need to get 1.1.0 out the door. I'm going to work on some of our internal toolchain stuff until I hear back.

[bmuenzenmeyer]

Great!

@geoffp thanks for the offer of help with 1.1.0 - I definitely appreciate and welcome it.

Could toy please take the README content stubbed out in #223 (comment) as a base and see if anything else is needed/requires change?

[geoffp]

On it.

[geoffp]

Okay -- I've gone through a "fresh install" from scratch and tried to piece together a fresh require()able copy of patlab, documenting each step I had to take.

This is going to bring into focus why it's not quite yet a supported feature, if you know what I mean. 😀 But it should also tell us what we need to streamline to make it a better supported configuration. Full markdown coming up next. I have the .md stored on my machine as well, so I can hand it off properly if/when you want to publish these instructions.

[bmuenzenmeyer]

👍 @geoffp thanks so much!

[geoffp]

@EvanLovely, would you review this as well, please?

EDIT: Forgot the create public dir. step.

Installing and running Pattern Lab as an NPM dependency

Note: support for this configuration is still young, and is still being actively worked on. Expect a few bumps in the road and bear with us as we polish this up.
Prerequisites: Node/NPM, and a project with gulp and/or grunt installed. Commands are given in bash.

1. Install Pattern Lab: Go to your project root and run npm install --save-dev patternlab-node
2. Copy the default config: you can call it whatever you want, but in this example, we do cp node_modules/patternlab-node/config.json patternlab-config.json
3. Copy the default source: you need a basic copy of the patternlab source files to get started. Do cp -r node_modules/patternlab-node/source .
4. Create the public and public/patterns folders: on a Unix machine, that's mkdir public; mkdir public/patterns
5. Copy the Gulpfile or Gruntfile: Either cp node_modules/patternlab-node/gulpfile.js . or cp node_modules/patternlab-node/Gruntfile.js .
6. Edit your config:
   1. In the path section at the top, tell Pattern Lab where to build its static HTML site; the default is [project root]/public.
   2. Add a styleguide entry to paths.source to tell gulp where to copy styleguide assets from. By default this should be ./node_modules/patternlab-node/public/styleguide
7. (Gulp only) Edit your gulpfile:

   1. Install NPM dependencies needed for the gulpfile: npm install --save-dev del gulp-strip-banner gulp-header gulp-nodeunit gulp-load browser-sync

   2. Edit the paths() function to point to patternlab-config.json or whatever you chose to call your custom config file.

   3. Comment out this line, currently at or near line 29: gulp.loadTasks(__dirname+'/builder/patternlab_gulp.js'); We need our own version of the patternlab task, not the default that's included.

   4. Create a custom patternlab task that loads your project's pattern lab config and passes it in. Something like:

      // The config and the Pattern Lab itself
      var config = require('./patternlab-config.json'),
          pl = require('patternlab-node')(config);
      
      gulp.task('patternlab', ['prelab'], function (done) {
        pl.build(true);
        done();
      });

   5. Create a task to copy over the styleguide assets. Something like:

      // Styleguide assets copy from node module to public dir
      gulp.task('cp:styleguide', ['clean'], function () {
        return gulp.src(path.resolve(paths().source.styleguide, '**/*'))
          .pipe(gulp.dest(paths().public.styleguide))
          .on('end', function () {
            console.log('Done copying styleguide stuff.');
          });
      });

   6. Add your styleguide copy task to the prelab task, as in:

      gulp.task('prelab', ['clean', 'assets', 'cp:styleguide']);

   7. Run it and see if it works!

      gulp serve

8. (Grunt only) Edit your Gruntfile:
   1. TODO

[EvanLovely]

Hey, sorry for my lack of responses — was on a birthday ski vacation :) I'll check into this very soon; thanks!!