Pass extensions key through parent argument #12

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
Contributor

andreypopp commented Mar 22, 2013

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 added some commits Mar 22, 2013

@andreypopp andreypopp resolve now accepts opts object
opts now can contain 'extensions' key which will be passed directly to
resolve function from resolve package. This allows to resolve non-js modules.
a4f23d2
@andreypopp andreypopp include fixtures for coffee cc0a233
@andreypopp andreypopp pass extensions through parent
we don't need separate optionsargument
fbc5b08
Contributor

andreypopp commented Mar 26, 2013

Any comment on this?

Contributor

andreypopp commented Mar 27, 2013

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.

@shtylman @substack Several dependant projects have issues requesting that this be fixed. Any comment on what is incorrect with shtylman#12

andreypopp referenced this pull request May 3, 2013

Closed

options passed to resolve #20

grassick commented May 6, 2013

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

nateps commented May 8, 2013

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

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

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

Contributor

andreypopp commented May 14, 2013

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

nateps commented May 22, 2013

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.

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

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

👍

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 Jun 4, 2013

You can try also webmake-coffee

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

Owner

defunctzombie commented Jun 11, 2013

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.

Contributor

andreypopp commented Jun 11, 2013

@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

Owner

defunctzombie commented Jun 11, 2013

@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.

FWIW I don't believe browserify devs are keen to add this to core after all: jnordberg/coffeeify#1 (comment)

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

Contributor

andreypopp commented Jun 11, 2013

@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

Contributor

andreypopp commented Jun 11, 2013

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

+1

@maxtaco maxtaco pushed a commit to maxtaco/icsify that referenced this pull request Mar 5, 2014

@hurrymaplelad hurrymaplelad Update README.md with an example of the browserify --extension option
defunctzombie/node-browser-resolve#12 has been merged and released
9d57aca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment