search

browserify / browser-resolve

resolve function which support the browser field in package.json
MIT License
102 stars 70 forks source link

Gets a basic deep module reference test working with 1.1.0 node-resolve #54

Closed justinbmeyer closed 9 years ago

justinbmeyer commented 9 years ago

For #51, this uses the 1.1.0 node-resolve to make deep module references mappable.

It works by adding a pathFilter that checks browser for a matching key and returns that value. It handles if the user does or does not end the map with .js.

It includes a test.

Thanks!

defunctzombie commented 9 years ago

@justinbmeyer This is all that is necessary to make direct require work and be able to shim via browser field in the nested modules?

defunctzombie commented 9 years ago

Once you make the style changes, please squash into a single commit and reference the original discussion as you have. Great job on following this through!

justinbmeyer commented 9 years ago

@defunctzombie Yes, it seems like this is all that is needed as most of the hard work is now being done in node-resolve. I tested this against the workflow in CanJS (where CanJS has everything in dist/cjs) that uses browserify and it worked right.

I will hopefully get the pull request cleaned up for tomorrow. Thanks!

justinbmeyer commented 9 years ago

Updated. Thanks!

defunctzombie commented 9 years ago

published in v1.7.0

Thanks!

mantoni commented 9 years ago

Now this seems to break browserify for me when I resolve files that are not .js files. I'm using custom transforms to precompile hogan templates which stopped working. If I manually downgrade browser-resolve to v1.6 it works fine again. But v1.7 is picked up by the current browserify release.

justinbmeyer commented 9 years ago

@mantoni Can you add a breaking test? Or give a small breaking example. What error do you get? I can get in a patch asap.

mantoni commented 9 years ago

@justinbmeyer Thanks. Had to run away. Will try and isolate tomorrow.

mantoni commented 9 years ago

Narrowed it down to this: https://github.com/substack/node-resolve/issues/69 Please consider nailing the dependency to resolve@1.0.0 for the time being.