Skip to content

Conversation

@tbouron
Copy link

@tbouron tbouron commented Feb 9, 2016

This is a follow up of #221.

One thing important to notice: as patternlab can now be entirely required, both gulp and grunt (dev) dependencies need to pulled.

Here is how a downstream project can use patternlab:

  • Intall patternlab as dependency: npm install patternlab-node --save-dev
  • Copy the config.json file: cp node_modules/patternlab-node/config.json patternlab-config.json Here is an example that uses patternlab files for the styleguide but my project's patterns, data, image, etc:
{
  "paths" : {
    "source" : {
      "root": "./source/",
      "patterns" : "./source/_patterns/",
      "data" : "./source/_data/",
      "styleguide" : "./node_modules/patternlab-node/core/styleguide/",
      "patternlabFiles" : "./node_modules/patternlab-node/source/_patternlab-files/",
      "js" : "./source/js",
      "images" : "./source/images",
      "fonts" : "./source/fonts",
      "css" : "./source/css/"
    },
    "public" : {
      "root" : "./public/",
      "patterns" : "./public/patterns/",
      "data" : "./public/data/",
      "styleguide" : "./public/styleguide/",
      "js" : "./public/js",
      "images" : "./public/images",
      "fonts" : "./public/fonts",
      "css" : "./public/css"
    }
  },
  ...
}
  • If your are using gulp, here is the content of the gulpfile.js:
var gulp = require('gulp'),
    config = require('./config.json');

// Load gulp-build tasks to the current gulp and inject the local configuration
require('./builder/gulp-build')(gulp, config);

// Your gulp tasks go here
...

gulp.task('default', ['pl:lab']);
  • If your are using grunt, here is the content of the Gruntfile.js:
module.exports = function(grunt) {

    var config = require('./config.json');

    grunt.initConfig({
        // Your grunt configuration goes here
        ...
    });

    // Load grunt tasks to the current grunt and inject the local configuration
    require('./builder/grunt-build')(grunt, config);

    // Your grunt tasks go here
    ...

    grunt.registerTask('default', ['patternlab', 'copy:pl_main', 'copy:pl_styleguide']);
};

@EvanLovely
Copy link
Member

If people are going to require() these grunt/gulp files; it'll register all these tasks along with their tasks and we may naming collisions. I would propose namespacing all pattern lab tasks names with something like pl- so we'd then run gulp pl-serve instead of gulp serve.

Additionally, it may be advantageous to break this up into core and extras; it'd be great to get just the pattern lab related tasks.

@tbouron
Copy link
Author

tbouron commented Feb 10, 2016

@EvanLovely I Agree.

What about prefix the tasks by pl: so serve would become pl:serve? I think it's more consistent with the current tasks (espacially the cp:(css|js|img|font)).

To break even more, I propose to replace the patternlab_(gulp|grunt) by:

  • (gulp|grunt)-core.js for patternlab tasks
  • (gulp|grunt)-extra.js for extra tasks such as tests (well actually, we don't even need this task as it can be called directly via npm)
  • (gulp|grunt)-build.js for build tasks.

The (gulpfile|Gruntfile).js will then require (gulp|grunt)-build.js which will require the other 2 files.

WDYT?

@EvanLovely
Copy link
Member

Love it all! 👍🏼🍻😀

@tbouron
Copy link
Author

tbouron commented Feb 10, 2016

Cool, I'll do the changes.

@tbouron
Copy link
Author

tbouron commented Feb 10, 2016

So because of how grunt works (configuration over code), I cannot prefix the tasks as I did for gulp. Let's take the copy task, I can either do:

  • concat:pl:main
  • concat:pl-main
  • concat:main:pl
  • concat:main-pl

Any preference?

Also, do you want me to squash the commits once I'm finished? I didn't find information about within the CONTRIBUTING.md file

/cc @EvanLovely @bmuenzenmeyer

@tbouron
Copy link
Author

tbouron commented Feb 11, 2016

@EvanLovely @bmuenzenmeyer Done! I also updated the PR's description with grunt|gulp examples

@bmuenzenmeyer
Copy link
Member

wow @tbouron thanks so much for all of this. I've been silent the past few days and am busy the remainder of the week - but generally love the proposed improvements to the grunt/gulp harness.

👍 to the direct nodeunit usage - I hadnt thought through that!

Also thank you for updating the README

I need to review this more thoroughly, but for starters can you please:

  • redirect this PR to dev branch?
  • rename package.json to package.gulp.json and find out what happened to the grunt package.json The projects' default is still grunt for now.

@tbouron
Copy link
Author

tbouron commented Feb 11, 2016

@bmuenzenmeyer You're welcome, patternlab-node helps us to build our corporate pattern library so it's normal to contribute back :)

To answer to you points:

  1. can't change the target branch so I'll need to close this PR and create another one.
  2. that's intentional to have only one package.json. I put it in bold within the PR description but it has slip through apparently :) Let me explain it again: as patternlab can now be a real npm dependency, a downstream project will either use its grunt or gulp to build the pattern library. However, you don't know which it will use. That's why you need install all gulp and grunt plugins to cover this use-case. Actually, I just realise that grunt and gulp plugins should be moved as dependencies and not devDependencies. Does it make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants