Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PCHR-4049: Upgrade gulp #2804

Merged
merged 6 commits into from
Aug 6, 2018
Merged

PCHR-4049: Upgrade gulp #2804

merged 6 commits into from
Aug 6, 2018

Conversation

igorpavlov-zz
Copy link
Contributor

@igorpavlov-zz igorpavlov-zz commented Jul 31, 2018

Overview

This PR is upgrading Gulp to the version 4.0.

⚠️You will need to reinstall all npm dependencies
⚠️You will need to update your global gulp

npm uninstall gulp -g
npm i gulp@4 -g

However, if by any reason you don't want to install a global gulp you can use npx gulp <command>.

See also the following PRs where Gulp is also being upgraded by following the similar procedure as in this PR:

Technical Details

"gulp.parallel" and "gulp.series"

Gulp 4 requires to pass a function as a second parameter to gulp.task, instead of an array. It also only accepts 2 parameters which means you cannot list functions as arguments anymore.

// gulp.watch(watchPatterns, ['requirejs'])
gulp.watch(watchPatterns, gulp.parallel('requirejs'))

"gulp-sequence" dependency is not needed anymore

Instead of using "gulp-sequence", we now use built-in gulp.series.

// gulpSequence.apply(null, sequence)(cb);
gulp.series.apply(null, sequence)(cb);

Spawning extensions

We cannot simply transform this code by applying gulp.series because we need to wrap util calls into function so they are not called on file initiation:

gulp.task('watch', function (cb) {
  gulpSequence(
    utils.spawnTaskForExtension('sass:watch', tasks['sass:watch']),
    utils.spawnTaskForExtension('requirejs:watch', tasks['requirejs:watch']),
    utils.spawnTaskForExtension('test:watch', tasks['test:watch'])
// should be simply transformed into
gulp.task('watch', gulp.series([
  utils.spawnTaskForExtension('sass:watch', tasks['sass:watch']),
  utils.spawnTaskForExtension('requirejs:watch', tasks['requirejs:watch']),
  utils.spawnTaskForExtension('test:watch', tasks['test:watch'])

For this reason a function that builds such promise array was created:

function buildTaskPromises (taskNames) {
  return taskNames.map(function (taskName) {
    return utils.spawnTaskForExtension(taskName, tasks[taskName]);

// and used:
var watcherPromises = buildTaskPromises(['sass:watch', 'requirejs:watch', 'test:watch']);
var builderPromises = buildTaskPromises(['sass', 'requirejs', 'test']);

gulp.task('watch', gulp.series(watcherPromises));
gulp.task('build', gulp.series(builderPromises));

Async functions -> promises

You may not want to use async functions anymore and use promises instead. Example as seen here https://github.com/compucorp/civihr/pull/2804/files:

// gulp.task('sass', function () {
//  civicrmScssRoot.updateSync();
gulp.task('sass', civicrmScssRoot.update);

"gulp.watch"

As per the documentation https://github.com/gulpjs/gulp/blob/4.0/docs/API.md#gulpwatchglobs-opts-fn

var watcher = gulp.watch('js/**/*.js', gulp.parallel('concat', 'uglify'));
watcher.on('change', function(path, stats) {
  console.log('File ' + path + ' was changed');
});

watcher.on('unlink', function(path) {
  console.log('File ' + path + ' was removed');
});

The gulp.watch.on now returns a file path (string) and not a file object. So we need to change:

// gulp.watch(watchPatterns).on('change', function (file) {
//  test.single(file.path);
gulp.watch(watchPatterns).on('change', function (filePath) {
  test.single(filePath);

Build tested (deleted dist files and successfully rebuilt without changes)
Requirejs tested
Sass tested
Watch tested
Test tested
Backstopjs tested

@igorpavlov-zz igorpavlov-zz force-pushed the PCHR-4049-upgrade-gulp branch 2 times, most recently from dbed1a1 to 32f8c98 Compare July 31, 2018 13:09
@igorpavlov-zz igorpavlov-zz force-pushed the PCHR-4049-upgrade-gulp branch 2 times, most recently from d41b8d7 to de22366 Compare August 1, 2018 15:43
@igorpavlov-zz igorpavlov-zz force-pushed the PCHR-4049-upgrade-gulp branch from de22366 to d37f669 Compare August 1, 2018 15:43
@igorpavlov-zz igorpavlov-zz merged commit 71d02cc into staging Aug 6, 2018
@igorpavlov-zz igorpavlov-zz deleted the PCHR-4049-upgrade-gulp branch August 6, 2018 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants