tmslnz / gulp-shopify-theme

Shopify theme synchronisation during development
Apache License 2.0
24 stars 6 forks source link

optional CB #5

Closed shelldandy closed 6 years ago

shelldandy commented 6 years ago

So we can emit an event after the stream 💅

shelldandy commented 6 years ago

The main reason for this change is to be able to trigger browsersync refreshes after the upload is done like a boss:

const browserSync = require('browser-sync')
const watch = require('gulp-watch')

const reload = () => {
  browserSync.reload()
}

gulp.task('theme:watch', done =>
    watch(themeFiles)
      .pipe(theme.stream(undefined, () => {
        reload()
        done()
      }))
      .on('error', config.onError)
)

I have tested and works flawless

shelldandy commented 6 years ago

@tmslnz give this a look if you can, i'll also do a pull of your latest changes so the merge is easy for you, im sure this is useful 😁 since before the browsersync reload was not always consistent haha

tmslnz commented 6 years ago

Agree will have a look soon.

tmslnz commented 6 years ago

@mike3run I've had a look and tried this, but it overlaps Gulp's done per-file callback. Being called each time one file completes is not too useful (crazy browsersync reloads for example) and it causes errors when a large amount of files are changed at once. I've had Error: write callback called multiple times, likely due to the file.done function now calling the stream callback as well as your "cb".

I think you can do what you're trying to do without a callback, but with dependent tasks, like so:

// Note this is the "native" gulp.watch, not `gulp-watch` module.
gulp.watch( 'src/assets/js/**/*.js', [ 'reload-on-js' ] );
gulp.task('js', function () { return gulp.src(…).pipe(…).pipe( theme.stream() ) });
gulp.task('reload-on-js', ['js'], reload);

The above will hit reload only at the end of the js task, if that has theme.stream() in its pipes.

Alternatively, since today's release (1.1.1) you can register a listener for theme.on('done', handler) or theme.on('error', handler). These will fire when one or more files in a queue have finished uploading (or errored out).

tmslnz commented 6 years ago

Apologies my mistake in 1.1.1 we had a bug in the queue, fixed in 1.1.2.

Red my previous

and it causes errors when a large amount of files are changed at once. I've had Error: write callback called multiple times, likely due to the file.done function now calling the stream callback as well as your "cb". That’s not the case anymore, but the overlap with Gulp’s idiomatic completion callback still stands. Let me know if the task dependency example above works for you, as that should eliminate the need for a callback on stream.

Also: events!

shelldandy commented 6 years ago

I think it might be since you use gulp 3 and I use gulp 4, in which it works fine, also browsersync provides options to only do the refresh after a certain amount of seconds have passed:

https://browsersync.io/docs/options#option-reloadDebounce

browserSync.init({
        port,
        open: false,
        logConnections: true,
        logPrefix: 'Pixel2Html',
        proxy: `https://${config.shopify.shopName}.myshopify.com`,
        startPath: `?preview_theme_id=${config.shopify.themeId}`,
        reloadDebounce: 2000,
        injectChanges: false,
        https: {
          key: fakeCert.key,
          cert: fakeCert.cert
        }
      })
shelldandy commented 6 years ago

My full browsersync task is as follows:

const gulp = require('gulp')
const config = require('../gulp.config')
const browserSync = require('browser-sync')
const openBrowser = require('react-dev-utils/openBrowser')
const WebpackDevServerUtils = require('react-dev-utils/WebpackDevServerUtils')
const {prepareUrls, choosePort} = WebpackDevServerUtils

const DEFAULT_PORT = 3000
const HOST = '0.0.0.0'
const protocol = 'https'

const fakeCert = require('../server/createCert')

gulp.task('browser-sync', done => {
  choosePort(HOST, DEFAULT_PORT)
    .then(port => {
      if (port === null) {
        return
      }
      const urls = prepareUrls(protocol, HOST, port)
      browserSync.init({
        port,
        open: false,
        logConnections: true,
        logPrefix: 'Pixel2Html',
        proxy: `https://${config.shopify.shopName}.myshopify.com`,
        startPath: `?preview_theme_id=${config.shopify.themeId}`,
        reloadDebounce: 2000,
        injectChanges: false,
        https: {
          key: fakeCert.key,
          cert: fakeCert.cert
        }
      })
      if (openBrowser(urls.localUrlForBrowser)) {
        openBrowser(urls.localUrlForBrowser)
      } else {
        openBrowser(urls.localUrlForBrowser + `?preview_theme_id=${config.shopify.themeId}`)
      }
      done()
    })
})