browserify / browser-resolve

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

Pass extensions key through parent argument #12

Closed andreypopp closed 11 years ago

andreypopp commented 11 years ago

Underlying resolve package allows resolving modules against non-js files. I think this is important feature to also support browser field for those non-js files — it will allow to use coffeescript (and others like typescript or roy) with module-deps and then with browserify w/o requiring to specify extensions in require(...) calls which is much saner.

Regarding test cases — for now, I simply copied them from test/module.js and test/local.js and test/fixtures to test/module-coffee.js and test/local-coffee.js and test/fixtures-coffee correspondingly.

andreypopp commented 11 years ago

Any comment on this?

andreypopp commented 11 years ago

Let me provide some clarification on this pull request — it basically changes just a single line by passing extensions down to resolve() call. Doing so we can use browser-resolve package just like resolve package. If you want me to squeeze commits into a single one — I can rebase that. If you have something against — please comment.

jackcviers commented 11 years ago

@shtylman @substack Several dependant projects have issues requesting that this be fixed. Any comment on what is incorrect with https://github.com/shtylman/node-browser-resolve/pull/12

grassick commented 11 years ago

+1 Being able to leave out .coffee extensions in browserify would be really nice.

nateps commented 11 years ago

+1 This is the expected interface for using coffee in requires, and can make coffee code unusable through browserify without modification

techpines commented 11 years ago

+1 This is a pain in the ass, just fix it! :)

jnordberg commented 11 years ago

+1 Agreed, I want to upgrade from browserify 1.x

alexgorbatchev commented 11 years ago

:+1:

alexgorbatchev commented 11 years ago

I resolved the merge conflicts in my branch https://github.com/alexgorbatchev/node-browser-resolve/commit/8fe560726da3e8a671abad4923c0d52e5c24a000

andreypopp commented 11 years ago

I also did a rebase and happy to reopen this pull request if @shtylman is willing to accept it :-)

nateps commented 11 years ago

This is a major annoyance, since it means that either you are forced to add .coffee in your requires, making it impossible to compile the code to js and run it, or you must always have a compiler running. Often, we'd like to develop without pre-compiling, but then compile before deploy. Please address.

yornaath commented 11 years ago

:+1: I want to do a similar thing with wisp https://github.com/Gozala/wisp

tiye commented 11 years ago

+1 Want it! CoffeeScript is a kind of life style and many people like it.

jackcviers commented 11 years ago

:+1:

alexgorbatchev commented 11 years ago

If you want to use browserify with coffee right now, i have a fully working fork with all modules ready to go at https://github.com/alexgorbatchev/node-browserify

npm install git://github.com/alexgorbatchev/node-browserify.git

bundle = browserify()
bundle.extension '.coffee'
medikoo commented 11 years ago

You can try also webmake-coffee

xdissent commented 11 years ago

If someone would merge this, I would be so happy.

defunctzombie commented 11 years ago

This commit is incorrect. There is no need for an "opts" parameter as the "parent" arg is the opts argument (I think this is clearer in the README.

andreypopp commented 11 years ago

@shtylman the first commit is incorrect, but I've fixed this in the next one, I can squash them all into a single commit if you are willing to accept this feature

defunctzombie commented 11 years ago

@andreypopp working on accepting some other pull request currently. If the only thing needed to make this work is passing opts.extensions then that is easy to do via the parent arg (maybe we can just rename it to "opt" :)

Yes, I will accept this feature. If you can merge the commit into one logical one so I can review and merge I would appreciate it.

xdissent commented 11 years ago

FWIW I don't believe browserify devs are keen to add this to core after all: https://github.com/jnordberg/coffeeify/pull/1#issuecomment-19232831

I don't see why it should not exist in this package, however.

andreypopp commented 11 years ago

@shtylman wow (x1000), this is so much awesome that I'm gonna buy you beer if we ever meet in person

I'll squash it into a single commit

andreypopp commented 11 years ago

@shtylman apparently GitHub doesn't pickup squashed commits from the same branch... so I've openned #25

lcrespom commented 11 years ago

+1