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

Apply transforms correctly to whole packages and modules that depend on multiple packages. #14

Closed
wants to merge 3 commits into from

Conversation

jaredhanson
Copy link
Contributor

This patch fixes the following two issues:

#7
#13

This depends on the following pull requests in dependencies of module-deps:

browserify/resolve#21
browserify/browser-resolve#23

Once in place, package metadata is preserved throughout the resolution process, allowing transforms to be applied correctly to whole modules (as well as addressing an issue of where transforms from multiple dependencies were not respected).

@ghost
Copy link

ghost commented Jun 9, 2013

I pulled this patch and am using your branch of browser-resolve#dir-replace but I get this error:

$ node test/tr_fn.js 

undefined:13
ction(require,module,exports){module.exports = function (x) { return x * GGG }
                                                                         ^
ReferenceError: GGG is not defined
    at /home/substack/projects/module-deps/test/files/tr_sh/node_modules/g/index.js.module.exports (eval at <anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
    at Array./home/substack/projects/module-deps/test/files/tr_sh/main.js../f.js [as 0] (eval at <anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
    at r (eval at <anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
    at /home/substack/projects/module-deps/test/files/tr_sh/f.js.module.exports (eval at <anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
    at eval (eval at <anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
    at Stream.<anonymous> (/home/substack/projects/module-deps/test/tr_fn.js:27:27)
    at Stream.EventEmitter.emit (events.js:123:20)
    at Stream.handleEnd (/home/substack/projects/module-deps/node_modules/duplexer/index.js:85:21)
    at Stream.EventEmitter.emit (events.js:123:20)
    at Stream.end (/home/substack/projects/module-deps/node_modules/browser-pack/index.js:52:14)

@jaredhanson
Copy link
Contributor Author

Hmm, and you've got node-resolve with the patches applied underneath that right?

Had all tests passing before submitting the PR. Away from my computer now, but I'll reverify when I'm back.

Sent from my iPhone

On Jun 8, 2013, at 6:14 PM, James Halliday notifications@github.com wrote:

I pulled this patch and am using your branch of browser-resolve#dir-replace but I get this error:

$ node test/tr_fn.js

undefined:13
ction(require,module,exports){module.exports = function (x) { return x * GGG }
^
ReferenceError: GGG is not defined
at /home/substack/projects/module-deps/test/files/tr_sh/node_modules/g/index.js.module.exports (eval at (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
at Array./home/substack/projects/module-deps/test/files/tr_sh/main.js../f.js [as 0](eval at %28/home/substack/projects/module-deps/test/tr_fn.js:27:9%29)
at r (eval at (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
at /home/substack/projects/module-deps/test/files/tr_sh/f.js.module.exports (eval at (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
at eval (eval at (/home/substack/projects/module-deps/test/tr_fn.js:27:9))
at Stream. (/home/substack/projects/module-deps/test/tr_fn.js:27:27)
at Stream.EventEmitter.emit (events.js:123:20)
at Stream.handleEnd (/home/substack/projects/module-deps/node_modules/duplexer/index.js:85:21)
at Stream.EventEmitter.emit (events.js:123:20)
at Stream.end (/home/substack/projects/module-deps/node_modules/browser-pack/index.js:52:14)

Reply to this email directly or view it on GitHub.

@ghost
Copy link

ghost commented Jun 9, 2013

The problem was just that resolve depended on 0.3.1 instead of 0.4.0. Silly mistake. Can you bump the resolve version on your dir-replace branch so I can depend on it in module-deps so we can push these fixes live without waiting for browser-resolve to get patched?

@ghost
Copy link

ghost commented Jun 9, 2013

Nevermind, I just cloned it myself. The new fixes are published as module-deps@0.10.2

@ghost ghost closed this Jun 9, 2013
@ghost
Copy link

ghost commented Jun 9, 2013

This patch removed opts.packageFilter which is now breaking browserify tests.

@jaredhanson
Copy link
Contributor Author

What are the failures?

I've got all browserify tests passing. Make sure you pull in the PR for browserify itself.

Sent from my iPhone

On Jun 8, 2013, at 8:25 PM, James Halliday notifications@github.com wrote:

This patch removed opts.packageFilter which is now breaking browserify tests.


Reply to this email directly or view it on GitHub.

@ghost
Copy link

ghost commented Jun 9, 2013

I added back opts.packageFilter and now everything works again. The bugs were in the field.js tests.

@jaredhanson
Copy link
Contributor Author

Whoops. Good catch.

This pull request was closed.
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

Successfully merging this pull request may close these issues.

None yet

1 participant