browserify / factor-bundle

factor browser-pack bundles into common shared bundles
Other
402 stars 27 forks source link

non-entry non-grouped modules don't rescue dependencies #5

Closed terinjokes closed 10 years ago

terinjokes commented 10 years ago

If a module is emitted by module-deps that doesn't match an entry file or groups, it is directly emitted on the common stream. It may, however, have dependencies that are only in a single group. In this situation, the dependencies will not be available at runtime.

ghost commented 10 years ago

It seems like your patch cleared this bug up. I wrote an extra test to be sure:

Input data:

[{ id: '/a.js', deps: { '/c.js': '/c.js', '/z.js': '/z.js' } },
{ id: '/b.js', deps: { '/c.js': '/c.js', '/x.js': '/x.js' } },
{ id: '/c.js', deps: { '/d.js': '/d.js' } },
{ id: '/d.js', deps: { '/e.js': '/e.js' } },
{ id: '/e.js', deps: {} },
{ id: '/x.js', deps: { '/y.js': '/y.js' } },
{ id: '/y.js', deps: { '/z.js': '/z.js' } },
{ id: '/z.js', deps: { '/c.js': '/c.js' } }]

Output for entry points /a.js and /b.js:

$ node lift.js
TAP version 13
# lift singly-shared dependencies
----- COMMON ROWS -----
[ { id: '/z.js', deps: { '/c.js': '/c.js' } },
  { id: '/c.js', deps: { '/d.js': '/d.js' } },
  { id: '/d.js', deps: { '/e.js': '/e.js' } },
  { id: '/e.js', deps: {} } ]
ok 1 should be equivalent
----- /a.js ROWS -----
[ { id: '/a.js', deps: { '/c.js': '/c.js', '/z.js': '/z.js' } } ]
ok 2 should be equivalent
----- /b.js ROWS -----
[ { id: '/b.js', deps: { '/c.js': '/c.js', '/x.js': '/x.js' } },
  { id: '/x.js', deps: { '/y.js': '/y.js' } },
  { id: '/y.js', deps: { '/z.js': '/z.js' } } ]
ok 3 should be equivalent

1..3
# tests 3
# pass  3

# ok
terinjokes commented 10 years ago

No, I don't think any of the patches so far fix this, and it's exacerbated by the patch added in #8:

/home/terin/local/cf-repos/cf-www-next/node_modules/factor-bundle/index.js:103
            Object.keys(row.deps).forEach(function(k) {
                           ^
TypeError: Cannot read property 'deps' of undefined
    at Factor._flush (/home/terin/local/cf-repos/cf-www-next/node_modules/factor-bundle/index.js:103:28)
    at Array.forEach (native)
    at Factor._flush (/home/terin/local/cf-repos/cf-www-next/node_modules/factor-bundle/index.js:98:11)
    at Factor.<anonymous> (_stream_transform.js:130:12)
    at Factor.g (events.js:175:14)
    at Factor.EventEmitter.emit (events.js:92:17)
    at finishMaybe (_stream_writable.js:354:12)
    at endWritable (_stream_writable.js:361:3)
    at Factor.Writable.end (_stream_writable.js:339:5)
    at Stream.onend (stream.js:79:10)

The modules that aren't entries and aren't in a group are still outputted directly on the output stream, but might later be requested from the _buffered array. (at least that's what I understand so far).

/cc @edrex

terinjokes commented 10 years ago

Added a failing test with terinjokes@1a7ffef.