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

9.0.6 breaks my gulp + watchify setup #1191

Closed
flowerornament opened this issue Apr 3, 2015 · 12 comments
Closed

9.0.6 breaks my gulp + watchify setup #1191

flowerornament opened this issue Apr 3, 2015 · 12 comments

Comments

@flowerornament
Copy link

@Gulpfile.js

var gulp       = require('gulp');
var gutil      = require('gulp-util');
var transform  = require('vinyl-transform');
var debug      = require('gulp-debug');
var browserify = require('browserify');
var watchify   = require('watchify');
var reactify   = require('reactify');
var _          = require('underscore');

var paths = {
  src:  './www/src/',
  dist: './www/dist/'
};

var b = browserify(paths.src + 'js/', _.extend(watchify.args, {debug: true}));
var bundler = watchify(b)
  .transform(reactify)
  .on('update', bundle)
  .on('log', console.error);

console.log(watchify.args);

function bundle() {
  var bundle = transform(function(filename) {
    return bundler.bundle();
  });
  return gulp.src([paths.src + 'js/index.js'])
    .pipe(debug({title: 'bundle'}))
    .pipe(bundle)
    .on('error', gutil.log.bind(gutil, 'Browserify Error'))
    .pipe(gulp.dest(paths.dist));
}

gulp.task('browserify', bundle);

Upgrading to 9.0.6 results in the following error:

_stream_readable.js:540
    var ret = dest.write(chunk);
                   ^
TypeError: undefined is not a function
    at Producer.ondata (_stream_readable.js:540:20)
    at Producer.emit (events.js:107:17)
    at Producer.Readable.read (_stream_readable.js:373:10)
    at flow (_stream_readable.js:750:26)
    at resume_ (_stream_readable.js:730:3)
    at _stream_readable.js:717:7
    at process._tickCallback (node.js:355:11)

9.0.4 works fine

@valette
Copy link

valette commented Apr 3, 2015

+1 here :

var browserify = require('browserify'),
    gulp       = require('gulp'),
    transform  = require('vinyl-transform'),
    uglify     = require('gulp-uglify');

gulp.task('default', function () {
    var browserified = transform(
        function(filename) {
            return browserify(filename).bundle();
        });

    return gulp.src([__dirname + '/lib/browserified.js'])
        .pipe(browserified)
        .pipe(uglify())
        .pipe(gulp.dest(__dirname + '/cache'));
});

@terinjokes
Copy link
Contributor

b.bundle() has always been documented* as returning a readable stream. From version to version, sometimes a duplex stream would be returned as an implementation detail, but writing to it was always undefined behavior.

If you must use gulp for this task, use vinyl-source-stream instead of vinyl-transform.

var browserify = require('browserify'),
    gulp       = require('gulp'),
    source     = require('vinyl-source-stream'),
    buffer     = require('vinyl-buffer'),
    uglify     = require('gulp-uglify');

gulp.task('default', function () {
    return browserify([__dirname + '/lib/browserified.js']).bundle()
        .pipe(source('app.js'))
        .pipe(buffer())
        .pipe(uglify())
        .pipe(gulp.dest(__dirname + '/cache'));
});

*Effectively always, starting with version 2.

@jmm
Copy link
Collaborator

jmm commented Apr 3, 2015

@terinjokes You probably already know this, but it's worth noting that the gulp project is still promoting this to the hilt:
gulpjs/gulp#970
gulpjs/gulp#989

@flowerornament
Copy link
Author

@jmm please keep the pressure on!

@terinjokes
Copy link
Contributor

I submitted a PR to fix the gulp documentation. I was unaware it was using
vinyl-transform in an undefined way.
On Apr 3, 2015 8:54 AM, "Morgan Sutherland" notifications@github.com
wrote:

@jmm https://github.com/jmm please keep the pressure on!


Reply to this email directly or view it on GitHub
#1191 (comment)
.

@jmm
Copy link
Collaborator

jmm commented Apr 3, 2015

@msutherl On gulp about recommending that technique? I'll do what I can, and as you can see @terinjokes submitted a PR regarding it.

@flowerornament
Copy link
Author

@jmm drawing attention to the issue toward a universally-endorsed solution but, alas, saw the PR

@jmm
Copy link
Collaborator

jmm commented Apr 9, 2015

@msutherl Oh, gotcha. Yes, I'd like to see this saga become as solved as it needs to be. I'm experimenting to see if there are abstractions that are worthwhile (compared to the kind of stuff presented in #1044) and feasible to wrap up in a module (an integration layer or "plugin").

@terinjokes
Copy link
Contributor

Two gulp recipes were corrected and an additional one was written last week.
On Apr 9, 2015 5:54 AM, "Jesse McCarthy" notifications@github.com wrote:

@msutherl https://github.com/msutherl Oh, gotcha. Yes, I'd like to see
this saga become as solved as it needs to be. I'm experimenting to see if
there are abstractions that are worthwhile (compared to the kind of stuff
presented in #1044
#1044) and feasible
to wrap up in a module (an integration layer or "plugin").


Reply to this email directly or view it on GitHub
#1191 (comment)
.

@zertosh
Copy link
Member

zertosh commented Apr 11, 2015

@terinjokes: Over at watchify, I've gotten a few issues where users have been re-adding transforms in their "rebundle" functions instead of when they first setup their browserify+watchify instance (see browserify/watchify#187). These aren't the same recipes you're talking about, right?

bakape added a commit to bakape/shamichan that referenced this issue Apr 17, 2015
Addreses undefined behavior, as described in browserify/browserify#1191
A less obfuscated system, now that the server does not rebuild the client on startup.
@thepian
Copy link

thepian commented Sep 6, 2015

Am I reading this wrong? It seems the objective is to produce multiple bundles and your fix is to make a single bundle.

@jmas
Copy link

jmas commented Sep 15, 2016

@terinjokes In your example above you suggest to put a file path to browserify(). But goal when we use vinyl-transform is to get a list of files by pattern and after build write them all to destination directory.
Pattern example: gulp.src(['directory/[^_]*.jsx', 'directory/[^_]*.js']).
In your current example we can't do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants