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

Using parcelify with Grunt #39

Open
prayagverma opened this issue Sep 20, 2015 · 9 comments
Open

Using parcelify with Grunt #39

prayagverma opened this issue Sep 20, 2015 · 9 comments

Comments

@prayagverma
Copy link

I am using grunt-browserify and trying integrate parcelify with it.

I used the the preBundleCB task option provided by grunt-browserify and created a grunt configuration like

var browserify = require('browserify');
var parcelify = require('parcelify');
module.exports = function(grunt) {
grunt.loadNpmTasks('grunt-browserify');
var taskConfig = {
    pkg: grunt.file.readJSON('package.json'),
    browserify: {
        main: {
            src: 'entry.js',
            dest: 'bundle.js'
        },
        options: {
            browserifyOptions: {debug:true},
            preBundleCB: function(b){
                return parcelify(b,{});
            }
        }
    }
};
grunt.initConfig(grunt.util._.extend(taskConfig));
};

This task doesn't fail but the parcelify plugin doesn't generate the CSS file.

But when running it from command line , it works without issue , I used the following command

browserify entry.js -o bundle.js -p [ parcelify -o bundle.css ]

After investigating , I found that the callback passed as parameter to _this.instantiateParcelAndPackagesFromMap ( https://github.com/rotundasoftware/parcelify/blob/v2.1.0/index.js#L122 ) function never gets executed.

I have working version of my code at https://github.com/pra85/grunt-browserify-example

Any help or advice for this issue would be really appreciated , Thanks

@dgbeck
Copy link
Member

dgbeck commented Sep 21, 2015

Hi @pra85 !

Unfortunately I can't offer very much help here as I don't think this is a parcelify issue and I have too much going on to try to reproduce, etc. Couple things came to mind..

  1. the bundle.css might be getting written to a different directory, since the working directory might change when not running browserify from the command line. Or, there might be some other issue related to the different in working directory.
  2. It looks like configure might be better to use than preBundleCB, since preBundleCB is called once for every bundle command. Probably not the issue here though.

https://github.com/jmreidy/grunt-browserify#configure

  1. If as you say the callback to this.instantiateParcelAndPackagesFromMap is not getting invoked, it would be helpful to track down exactly where things are going awry.
  2. Maybe try listen for error events coming from the parcelify event emmitter:
preBundleCB: function(b){
  var p = parcelify(b,{});
  p.on( 'error', function( err ) { console.log( "Parcelify error!" + err ); } );
}

I hope that helps at least a little !

David

@poonwu
Copy link

poonwu commented Oct 13, 2015

Hi @pra85 ,

I have researched on this issue, it seems the grunt-browserify is causing async.setImmediate to not invoked properly. I tried a workaround by forcing the async.setImmediate to fallback by undefining global.setImmediate.

example

Of course, this mean that if you have other package that depends on global.setImmediate, that package will surely break.

@mediafreakch
Copy link

From what I can tell this issue is related to the one I'm facing with karma-browserify.

Both karma-browserify (here) and grunt-browserify (here) add the files to be bundled using b.require. Like that, browserify doesn't set { entry: true }, which causes parcelify to drop the parcel/package (here).

How can we make this work? Is there a specific reason that parcelify is relying on entry: true?

Edi

@dgbeck
Copy link
Member

dgbeck commented Mar 25, 2016

Hi @mediafreakch !

Great to know that the issues are related.

Is there a specific reason that parcelify is relying on entry: true?

I'm not sure any other way to determine which rows in the browserify pipeline correspond to entry points, and parcelify needs that information to do its thing. If there is another way, I'm open to switching.

But it seems clear that the entry property is expected to be true iff the row corresponds to an entry point. Since it not behaving according to that expectation, there is clearly a bug in browserify. I think the best way to resolve this issue is at that level, with either a bug report or a PR on the browserify repo.

@mediafreakch
Copy link

@dgbeck I have been trying to understand the parcelify process, without much luck. I'm stuck at what the role of an entry file is in parcelify... Can you help? Why is { entry: true } mandatory? Can't parcelify just resolve the dependency tree regardless?

@dgbeck
Copy link
Member

dgbeck commented Apr 13, 2016

Hi @mediafreakch,

Parcelify needs to know which row is the entry point so that it can figure out where the "top" of the js dependency graph is, so that it can build the css bundle by concatenating css files in the correct order.

Thinking this through further, in most cases it would be possible to look at the structure of the dependency tree after it has been built, and figure out where the "top" is by finding the node on which nothing depends, but that approach will not work if something depends on the entry point. (Also there is a case of multiple entry points to consider, although in that case I think we could use the keys in bundlesByEntryPoint option to figure out which files are entry points.)

Again, this issue has highlighted a clear, reproducible bug in browserify. Rather than jumping through hoops to try to work around it in parcelify, the cleaner approach is to fix the issue upstream at its root. Have you submitted a issue on the browserify repo?

@mediafreakch
Copy link

@dgbeck I see. The thing with the order makes sense.

I haven't submitted an issue because I don't see the reproducible bug you mention.
The browserify documentation explicitly says that there is a b.add method that is used to set an entry point and on the other hand a b.require method that doesn't. It also seems that b.add is just a shorthand for b.require(file, { expose: false }).

As I understand this is by design to allow bundles that have no entry-point (and require them in other bundles).

@dgbeck
Copy link
Member

dgbeck commented Apr 14, 2016

Ah, ok, I see.

I'm having a hard time figuring out where the root of the problem in light of that information.

To me it seems like if there is no entry point, it makes no sense for parcelify to bundle anything. I'm confused as to why karma-browserify and grunt-browserify are neglecting to "specify" an entry point, when it seems clear that conceptually there is a entry point in these cases. I'm inclined to think that the root of the problem therefore is in the way these repos are interfacing with browserify.

However, if this issue is blocking you, I realize that doesn't help much, since I have a feeling those repos are not going to change the way they are doing things.

A work around that comes to mind, would be to "tell" parcelify what the entry point is using the bundlesByEntryPoint map. Do you have the absolute path of your entry point? If so, you could include an entry for that single entry point in that option, and then parcel-map could treat the row that matches that path as an entry point. To make this work, parcelify would need to pass the keys of bundlesByEntryPoint to parcel-map, and then when parcel-map does its checks (on like 62 and 73) to determine whether or not a row is an entry point, it would scan the list of entry points provided to it by parcelify (from the bundlesByEntryPoint option), and treat the matches as entry points. For example, on row 62,

if( row.entry || opts.entryPoints && _.contains( opts.entryPoints, thisFilePath ) ) {

kind of a dirty work around, but whatever. I'm open to a PR for that.

@mediafreakch
Copy link

I'm waiting for nikku/karma-browserify#182 to be reviewed. If that gets approved, the only thing left would be for karma-browserify to wait for parcelify to do its job before launching the tests.

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

No branches or pull requests

4 participants