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

Suggested updates to 'Browsersync + Gulp' documentation #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

justinhelmer
Copy link

The documentation is potentially misleading. Gulp dictates that a task must do one of the following:

  1. accept a callback
  2. return a stream
  3. return a promise

...in order for the task to behave asynchronously.

I found myself spending far more time than desired debugging an issue that could have been solved if I had just a little bit more knowledge.

The suggestion is to make a few minor updates to the documentation, to save the heartache for future consumers that I had to experience.

The changes in this pull request are as follows:

  • For all tasks that don't return streams, supply a callback.
  • Add an inline comment to the code block(s) for the very first example.
  • Add a sentence explaining the necessity for ensuring the task is asynchronous, after the very first example.

@revelt
Copy link
Contributor

revelt commented Feb 9, 2017

I'm migrating my Gulp tasks to Gulp v4 and ES2015 (via Babel) and have to ensure that all all tasks are signalled as completed, usually using promises. Otherwise, you get an error:

async_completion

I'm trying to set up bs.init, and I wanted to check in the API docs, what is it actually returning, but there's not mention about it in documentation.

For example, can we put resolve() in the callback on the bs.init like that:

export function syncBrowser () {
  return new Promise(function (resolve) {
    bs.init({
      server: {
        baseDir: './dist',
        index: 'index.html'
      }
    }, function () {
      resolve()
    })
  })
}

?

It would be helpful if API documentation covered the Gulp v4 context as more and more people will be migrating their gulpfiles.

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