caleb / broccoli-fast-browserify

A caching, flexible Browserify Broccoli filter
https://github.com/caleb/broccoli-fast-browserify
BSD 3-Clause "New" or "Revised" License
14 stars 7 forks source link

Fast-Browserify does not pick up some files? (perhaps symlinks?) #15

Open ccoenen opened 9 years ago

ccoenen commented 9 years ago

I made a repository with exactly three commits, you can clone it here: https://github.com/ccoenen/broccoli-fast-browserify-issue-next

On Linux, this happens:

I do not have this issue on windows. On windows, all three commits work! I have the hunch that fast-browserify does not pick up on symlinks. The previous thing in the brocfile uses caching-writer, and that seems to symlink unchanged files.

ccoenen commented 9 years ago

I have this hunch mainly, because fast-browserify tries to find react-component in app - but this is not where it's supposed to look! It's supposed to look in the directory specified by the previous output-tree, which would probably be something in /tmp/...

ccoenen commented 9 years ago

It's not the symlink. I modified all instances of symlink-or-copy to just copy and the behaviour is still the same. Browserify itself gets intsanciated with the right path (tmp...), but somewhere along the way it still gets lost.

caleb commented 9 years ago

Thanks for the detailed report. I will check it out when I get a chance.

ccoenen commented 9 years ago

i'll keep you posted with the debug progress in here. Maybe i'll figure it out. Or i'll find a workaround along the way :D

ccoenen commented 9 years ago

The very same thing happens, when i convert to ES6 imports and run my app.js bundle through transform: babelify.

ccoenen commented 9 years ago

it works just fine with "regular" JS Files that are in /app anyway. But the "here's a JSX, and now it's a JS" move appears to be confusing something. I'll stop debugging for now.

jfgodoy commented 9 years ago

@ccoenen, I have the same issue.

Starting with this specific commit https://github.com/substack/node-browserify/commit/504832b4b9fde30c36877122b265ddccbea7be77, browserify resolve symlinks.

Broccoli after a transformation create a new temporal folder with symlinks to unchanged files, in your case app.js.

So browserify resolve app.js first to the original app/app.js, then in that folder search the relative path for ./react-component, which does not exists.

The first work around, is to set browserify version in this plugin to 10.2.4.

I don't know if the new behavior in browserify is a bug.

jfgodoy commented 9 years ago

Here another simple example of the problem, Consider 2 files: public/need_transform/a.js

console.log('hello');

public/index.js

require('./transformed/a.js')

The idea is with some broccoli plugin transform the file in need_transform folder an put in transformed folder. Then, using this plugin, merge the scripts into a single file.

Brocfile.js

var mergeTrees = require('broccoli-merge-trees');
var fastBrowserify = require('broccoli-fast-browserify');
var funnel = require('broccoli-funnel');

// the folder public/need_transform is tranformed using some plugin,
// in this example, I just rename the folder
var transformed = funnel('public', {srcDir: 'need_transform', destDir: 'transformed'});

// the transformed folder is merged with the original folder. Broccoli will create
// a new temporal folder with symbolic links to the original files
var scripts = mergeTrees(['public', transformed]);

// the compilation with browserify 10.2.5+ fails. The reason is that the symbolic link of
// index.js is resolved to public/index.js, then the relative path inside require folder is resolved
// to public/transformed/a.js which does no exists 

var compiled = fastBrowserify(scripts, {
  bundles: {
    'app.js': {
      entryPoints: 'index.js'
    }
  }
});

module.exports = compiled;
caleb commented 9 years ago

I wish I had time to debug this, but I'm in a crunch period at work now. It may be a couple weeks before I can track this down, sorry!

If you find anything submit a PR and I can make a new release :)

caleb commented 9 years ago

Thanks to @jfgodoy for finding the problem. I've published a new version of broccoli-fast-browserify with the version of Browserify locked to 10.2.4. Apparently they changed the way browserify works with symlinks.

I'm busy for a couple weeks, but I can take a look at then if there isn't a PR that fixes it :)

Thanks for all the debugging!

ccoenen commented 9 years ago

I can confirm that it's no longer an issue with 0.2.8.

Would you like to keep this open for a more permanent fix?

Big thanks to @jfgodoy for your support, too!

caleb commented 9 years ago

Yeah, I was planning on leaving this open. I should add something to the README as well.

elldritch commented 8 years ago

:+1: Any update on this? This is by far the best of the Broccoli + Browserify integrations.

caleb commented 8 years ago

I might have time to look into this on Tuesday.

stefanpenner commented 8 years ago

https://github.com/eploko/broccoli-watchify 1.0.0 now addresses this issue via https://github.com/eploko/broccoli-watchify/commit/f8debb5d0c1672bae14bef802e93867c44b00216