cloudfour / core-gulp-tasks

⛔️ DEPRECATED Gulp tasks that we usually need
MIT License
9 stars 0 forks source link

Make js task easier to set up 💡 #55

Open tylersticka opened 7 years ago

tylersticka commented 7 years ago

One pet peeve I have on our projects is having to manually create new entry point configurations for our js task.

On a recent project I discovered that it's possible to mimic the behavior of our other tasks using webpack-stream and vinyl-named.

Here's an example that doesn't take into account any of the existing requirements, but hopefully demonstrates the possibilities:

var gulp = require('gulp');
var changed = require('gulp-changed');
var named = require('vinyl-named');
var webpack = require('webpack-stream');
var browserSync = require('browser-sync').create();

var SRC = 'src';
var DEST = 'dist';

gulp.task('js', () => {

  /**
   * 1. Process every JS file in the root of the scripts directory.
   * 2. Only process files that have changed since the last time we did this.
   * 3. Name these files for webpack. Default behavior is basename minus ext.
   * 4. Process files with webpack.
   * 5. Dump output to destination.
   * 6. Notify BrowserSync of anything that changed.
   */

  return gulp.src(`${SRC}/scripts/*.js`) /* 1 */
    .pipe(changed(DEST)) /* 2 */
    .pipe(named()) /* 3 */
    .pipe(webpack()) /* 4 */
    .pipe(gulp.dest(DEST)) /* 5 */
    .pipe(browserSync.stream()); /* 6 */
});

The reason I'm making an issue rather than just submitting a PR is that this would likely break configuration for existing projects, so it's not something I'd want to pursue without some discussion.

tylersticka commented 7 years ago

Just learned that gulp-changed is not ideal because it won't respond accurately to require'd assets (like JSON files). Had more luck with this variation:

var gulp = require('gulp');
var gutil = require('gulp-util');
var named = require('vinyl-named');
var webpack = require('webpack-stream');
var browserSync = require('browser-sync').create();

var SRC = 'src';
var DEST = 'dist';

gulp.task('js', callback => {
  var firstBuildReady = false;
  var resolved = false;

  gulp.src(`${SRC}/scripts/*.js`)
    .pipe(named())
    .pipe(webpack({
      watch: gutil.env.dev // or whatever
    }, null, (err, stats) => {
      gutil.log(stats.toString({
        colors: gutil.colors.supportsColor,
        chunks: false,
        hash: false,
        version: false
      }));
      browserSync.reload();
      firstBuildReady = true;
    }))
    .pipe(gulp.dest(DEST))
    .on('data', () => {
      if (firstBuildReady && !resolved && callback) {
        resolved = true;
        callback();
      }
    });
});
calebeby commented 6 years ago

@tylersticka Should this be implemented, or should I replace webpack copy task for js files, as Erik mentioned here

tylersticka commented 6 years ago

@calebeby I'm not sure. Could you summarize the pros/cons for me of the approaches we could take? When it comes to JavaScript build tools I feel like I'm mostly copying and pasting examples. 😕

calebeby commented 6 years ago

I looked at this some more, and even though switching to copying the scripts would be simpler for drizzle itself, pattern libraries based on drizzle often have many outside dependencies and a more complex dependency tree. I think that it wouldn't be possible to use a copy task in that case.

calebeby commented 6 years ago

@tylersticka what do you want to happen with scripts that are in a subdirectory inside of assets/toolkit/scripts/?

Should they get their own separate bundle generated, or should we assume they are a dependency of another bundle, and ignore them?

tylersticka commented 6 years ago

Should they get their own separate bundle generated, or should we assume they are a dependency of another bundle, and ignore them?

@calebeby This one (I think) ☝️

(Assuming that makes sense to you and to @gerardo-rodriguez)

tylersticka commented 6 years ago

@calebeby Actually, it'd be kinda nice if we could do something similar to what we do for a lot of projects with our styles/ directory, where the root of styles/ gets compiled but we can optionally also compile a subdirectory like styles/prototypes/. Would that be a huge pain?

calebeby commented 6 years ago

Proposal for js option in config.js:

js: {} // don't pass anything, defaults to "./src/assets/*/scripts/*.js"
js: {
  entry: "./src/assets/toolkit/scripts/*.js" // pass in a glob, it will override the defuault
}
js: {
  entry: [ // pass in an array of globs, it will override the defuault
    "./src/assets/drizzle/scripts/*.js",
    "./src/assets/toolkit/scripts/*.js"
  ]
}

I think internally it will use Rollup instead of Webpack, because Rollup emits a smaller bundle and is generally easier to use (cloudfour.com-patterns is already using rollup).

@tylersticka Does this work the way you wanted?

@gerardo-rodriguez Does this look reasonable to you too?