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

Async configuration / gulpfiles (feature request) #1505

Closed
theoy opened this issue Jan 28, 2016 · 17 comments
Closed

Async configuration / gulpfiles (feature request) #1505

theoy opened this issue Jan 28, 2016 · 17 comments

Comments

@theoy
Copy link

theoy commented Jan 28, 2016

Background

AFAIK, when gulp loads configuration / discovers your tasks via a user project's gulpfile.js, it relies on side effects on the gulp object to decide which tasks were defined. This is very nice from a simplicity standpoint. And gulp has existing support for implementing tasks that execute asynchronously.

However, this makes it hard to define/declare tasks if the act of doing so relies on asynchronous APIs. For example, I have worked in some projects where we want to define our tasks deterministically from repository assets, but programmatically. To do so, we invariably call Node filesystem APIs, or execute a few child processes.

Right now, we can only do this on versions of Node that have synchronous versions of File I/O and child processes... and even then it would not use the ideal degree of possible concurrency. gulp is awesome about asynchrony and concurrency elsewhere, please let us do the same thing while defining tasks 😀

Similar proposal for grunt: gruntjs/grunt#783

Suggested Possible Design

Support asynchronous gulpfile.js in two forms:

Form 1: exported thenables (Promise)
module.exports = ... some thenable ...;
Form 2: exported function (that can return a thenable)
module.exports = function () {
    return ... some thenable ...;
}

If the exported value from gulpfile.js is a thenable (Form 1), then that thenable cooperatively signals when configuration is complete. If the exported value is a function (Form 2), then that function also gets invoked exactly once and if the return value is thenable then that value represents the completion of configuration.

The reason why Form 2 is important is because exporting a thenable by assigning to module.exports is tricky to do asynchronously. It's much more intuitive to return a promise chain from a function, because you can use the return statement in branches, loops, etc.

Using thenables makes gulp agnostic to whether the project is running on a V8 version with builtin Promise vs. using one of the many polyfills.

Optionally support a configuration timeout to help users realise that their async configuration code may have a bug when it doesn't complete before the timeout.

Compatibility

Technically changing the interpretation of gulpfile.js by observing returned functions/thenables is a compatibility break because existing versions of gulpfile.js may already be doing that and relying on gulp to ignore them. However, this is probably unlikely.

If you want to support this without a breaking change, you can add a config step API like gulp.config that can accept a thenable-returning function.

@phated
Copy link
Member

phated commented Jan 28, 2016

I'd probably just rename your gulpfile to gulpfile.babel.js and use async/await to pause execution until your async stuff is done.

@theoy
Copy link
Author

theoy commented Jan 28, 2016

I'd probably just rename your gulpfile to gulpfile.babel.js and use async/await to pause execution until your async stuff is done.

@phated - Hrm. I'm not sure that would work, because while async/await is great for implementing async functions, AFAIK the statements executed in the process of creating the module cannot be asynchronous. When I test this out with the babel plugins syntax-async-functions, transform-regenerator, then it errors if you try to use 'await' at the global / top-level scope.

SyntaxError: test.es6: Unexpected token (4:6)
  2 | 
  3 | var example = Promise.resolve();
> 4 | await example;
    |       ^

@theoy
Copy link
Author

theoy commented Jan 28, 2016

Also added compatibility section to OP as food for thought, depending on the compat philosophy used in this project 😄

@phated
Copy link
Member

phated commented Jan 28, 2016

@theoy
This should work fine:

async function config() {
  var isProd = await Promise.resolve(true);

  if (isProd) {
    gulp.task('build', prod);
  } else {
    gulp.task('build', dev)
  }
}

config()

@theoy
Copy link
Author

theoy commented Jan 28, 2016

@phated - I don't think your example works as you expect. Here's a counterexample based on your sample.

require("babel-polyfill");

var gulp = require('gulp');

async function config() {

    // await on a timeout for 3 seconds, for demonstration
    await new Promise(resolve => {
        setTimeout(() => {
            resolve();
        }, 3000);
    });

    // this gets registered after await returns after the timeout.
    gulp.task('default', () => {});
}

config()

I added the 3s delay just to make the timing obvious. What happens is that config() returns a Promise, but that Promise is not awaited before the require(...) call completes for gulp. For me, the above example causes gulpfile.babel.js to error because default is not registered in time before gulp looks for it.

@phated
Copy link
Member

phated commented Jan 29, 2016

Yeah, I'm not too familiar with async/await but thought you could use it in some way. This isn't something we are going to add to the API, especially now that ES2015 module syntax is supported through babel-commonjs interoperability. Sorry.

@phated phated closed this as completed Jan 29, 2016
@gregorskii
Copy link

gregorskii commented Aug 17, 2016

Hi there,

I am looking into this more as I have a use case where the configuration for gulp is defined in an async process.

Looking into the gulp.cli this line is stopping the config from being defined asynchronously:

https://github.com/gulpjs/gulp-cli/blob/master/lib/versioned/%5E3.7.0/index.js#L35

If the file were refactored in a similar way to this:

'use strict';

var chalk = require('chalk');
var log = require('gulplog');
var stdout = require('mute-stdout');
var tildify = require('tildify');

var taskTree = require('./taskTree');
var logTasks = require('../../shared/log/tasks');
var logEvents = require('./log/events');
var logTasksSimple = require('./log/tasksSimple');
var registerExports = require('../../shared/registerExports');

function run(opts, env, gulpInst, toRun) {
    if (opts.tasksSimple) {
      return logTasksSimple(env, gulpInst);
    }
    if (opts.tasks) {
      var tree = taskTree(gulpInst.tasks);
      tree.label = 'Tasks for ' + chalk.magenta(tildify(env.configPath));
      return logTasks(tree, opts.depth, function(task) {
        return gulpInst.tasks[task].fn;
      });
    }
    gulpInst.start.apply(gulpInst, toRun);
}

function execute(opts, env) {
  var tasks = opts._;
  var toRun = tasks.length ? tasks : ['default'];

  if (opts.tasksSimple || opts.tasks) {
    // Mute stdout if we are listing tasks
    stdout.mute();
  }

  // This is what actually loads up the gulpfile
  var exported = require(env.configPath);
  log.info('Using gulpfile', chalk.magenta(tildify(env.configPath)));

  var gulpInst = require(env.modulePath);
  logEvents(gulpInst);

  registerExports(gulpInst, exported);

  // Always unmute stdout after gulpfile is required
  stdout.unmute();

  // run function to setup gulp async, start on callback
  if (typeof exported === 'function') {
      exported(() => {
          run(opts, env, gulpInst, toRun);
      });
  } else {
      process.nextTick(function() {
        run(opts, env, gulpInst, toRun);
    });
  }
}

module.exports = execute;

Where the code checks the type of the exported gulpfile here and then waits for the resolution of that type (if its a thenable) before calling gulpInst.start here a gulpfile with a random delay would work correctly:

const gulp = require('gulp');

module.exports = (done) => {
    setTimeout(() => {
        let config = {
            a: 'value'
        }

        gulp.task('default', () => {
            console.log(config.a);
        });
    }, 5000);
};

Result from running:

→ gulp                                                                                                                                                                               [5fdb711]
(node:79483) fs: re-evaluating native module sources is not supported. If you are using the graceful-fs module, please update it to a more recent version.
[19:02:31] Using gulpfile ~/projects/test/gulpfile.js
[19:02:36] Starting 'default'...
value
[19:02:36] Finished 'default' after 209 μs

Where in this process gulp waited 5 seconds before executing default.

However I do see one issue the gulp autocompletions do not work as its likely not running through the same process as the CLI starter.

What do you guys think? It seems like it is conceivably possible to defer the start of the gulp process for async config. Understandable that this is not a common use case, but the code outlined above is a non breaking change as long as no gulp process is expecting the gulpfile to export a function or thenable already.

Thanks,

Greg

@phated
Copy link
Member

phated commented Aug 17, 2016

We still have no plans to add another API to handle this.

@gregorskii
Copy link

gregorskii commented Aug 17, 2016

Are there any alternative ways to handle this you can think of?

If there are no plans to add it can you provide a brief explanation as to why it is not an ideal feature?

EDIT: Wanted to elaborate a bit on why I would like to know this information. From the readme for v4.0 a comment states:

/* Not all tasks need to use streams, a gulpfile is just another node program
 * and you can use all packages available on npm, but it must return either a
 * Promise, a Stream or take a callback and call it
 */

I think the impetus for this comment in 4.0 is that the goal for gulp is not only to be a fast streaming build system as it was a couple years ago, but also to be a general purpose task runner.

I have been using gulp a lot lately for that purpose. I am using it on one project to organize some legacy java tasks that need a bit of prep work to run (in terms of arguments and file system preparation using fs) then execution via child_process.spawn. On another project I am using it to start docker, orchestrate maven job runs via child_process and also run webpack and a suite of testing tools. In both of these projects there is a heavy use of configuration and the speed at which they execute (within reason) is not a concern. A lot of the tasks on this project use callback based gulp.task method signature and wait for some async process to run. Gulp supports this out of the box, but I have found a use case where the setup required to run some of these tasks requires some async/thenable/callback based setup before running. The current use case I have for this is to read some properties from a pom.xml using fs.readFileSync, and then converting the poms project.properties to a JSON array using xml2js. xml2js unfortunately enforces its Parser.parse method to use a callback. Even if this callback is almost instantaneous it breaks the gulp cli runner because it is expecting that the gulp.tasks are defined immediately on the gulpfile (or via its immediate imports) and it can simply call the task you gave it as an argument after a process.nextTick like I detailed above.

Again it is understandable that these use cases are not normal for what gulp is intended for, but in order to make use of the best and simplest node task runner for other processes besides streaming builds I do see value in supporting a thenable config.

I would really appreciate an explanation of how this request diverges or is not in sync with the goals of the project so I can determine if I need to move a different direction in the future with these kinds of tasks.

Thank you very much.

@wwalser
Copy link

wwalser commented Sep 27, 2016

Just wanted to leave a note that I ran into this need as well.

Google's cloud infrastructure doesn't load metadata into the environment but requires you to call an external service. While async tasks can be used, I was hoping to implement the async config loading inside the gulpfile instead of inside each task.

I can do it inside each task. It's a bit of extra boilerplate that I don't want but it'll do.

Cheers.

@phated
Copy link
Member

phated commented Sep 27, 2016

I'm not really interested in solving everyone's problems (that's what StackOverflow is for); however, this can easily be solved by thinking outside the box a tiny bit.

function injectConfig(task) {
  return function(cb) {
    getConfig().then(function(config) {
      task(config, cb);
    });
  };
}

gulp.task('somethingThatNeedsConfig', injectConfig(function(config, cb) {
  // use config
  // do something async
  cb();
}));

@wwalser
Copy link

wwalser commented Sep 27, 2016

What you've suggested is more or less what I've done. My intention in commenting was not to persuade you to "solve everyone's problems" or even my own. I simply commented in order to provide feedback indicating that this is, in fact, a felt need. Think of it as a +1 with additional context on why the felt need exists. Nothing more.

@gregorskii
Copy link

gregorskii commented Sep 27, 2016

The use case I was describing before was a case where the actual tasks are defined based on an async process. It is not only that the task running needs async config, the entire gulp process requires async config.

The solution above is not even a change to gulp itself, its a change to the gulp-cli.

I don't quite understand the animosity towards this request. Its apparent that its a need in some complex configurations that use Gulp. It may not be the most common use case, but the Gulp team is not even considering the use cases for this configuration valid. The only options that remain for those who desire these changes are to use another tool, work around them, or fork gulp-cli. None of those solutions are the most ideal means of solving the ask.

@listenrightmeow
Copy link

listenrightmeow commented Sep 27, 2016

@phated if you're unwilling to work towards a more sensible solution than adding boilerplate to every task, can you point any of us in the direction of somebody that will? In this comment chain, there is a simple solution to the problem without the need of adding an API to handle the solve.

@wwalser
Copy link

wwalser commented Sep 27, 2016

I'd just like to say that I have absolutely no problem with the decision to not include this feature and that I don't support the "give us more free code" tone. It was not my intention to demand anything but rather to add a small tick of support to the request.

If anyone from gulp had indicated interest in the feature I would have created a PR. As it stands it looks like an undesired feature. No dramas.

@gregorskii
Copy link

gregorskii commented Sep 27, 2016

Just to be clear I don't believe in a "give us free code" mentality. If the feature request were accepted I would submit my own PR to solve it. My point above is about the outright animosity towards adding the feature in the first place. To be clear that is not simply because there was a disagreement in the feature being included in source, that is the Gulp teams prerogative to decide, but rather the animosity towards the idea as a whole when multiple people have requested it. I did not chime back in above until someone else requested the same feature, simply because I had accepted their answer and moved on.

If its not a desired feature, so be it. But that renders Gulp incapable of meeting certain requirements of projects, its quite sad that that forces people to fork or move in a different direction.

Personally I do not want drama either. But I don't think requesting a feature, and reinforcing that request when multiple people ask for it, would be drama.

@phated
Copy link
Member

phated commented Oct 11, 2016

Going to end this thread.

There's a few solutions but this isn't going into gulp core:

  • Use my solution above
  • Implement a custom CLI

@gulpjs gulpjs locked and limited conversation to collaborators Oct 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants