options passed to resolve #20

Closed
mikerobe opened this Issue May 3, 2013 · 14 comments

Comments

Projects
None yet
6 participants

mikerobe commented May 3, 2013

I ran into an issue with browserify, which uses browser-resolve, in that it wasn't finding .coffee files. It turns out this was because it (unlike node loaded with require('coffee-script')) requires the full extension -- require('foo.coffee') instead of require('foo'), which is definitely not something you want to do (it locks you into coffee files, won't use the .js if you "transpile" it, etc.)

Browser-resolve uses resolve and resolve has an option to add file extensions using the extensions key in its command options, but resolve's options aren't exposed in the browser-resolve API, so there's no way to add this file extension when using browser-resolve and thus no way to do it in browserify.

I think browser-resolve should pass the options it receives onto the internal resolve call by copying the keys over from parent.

Contributor

andreypopp commented May 3, 2013

see #12 and and #19

@mikerobe i have a browserify branch that should have this addressed https://github.com/alexgorbatchev/node-browserify

Owner

defunctzombie commented May 14, 2013

I will take a look at the rebased work. I will once again say tho that my
focus is on JavaScript development and what this proposes is not for
JavaScript development and therefore I see it as an anti-feature and am not
interested in it :)
On May 14, 2013 5:56 PM, "Alex Gorbatchev" notifications@github.com wrote:

@mikerobe https://github.com/mikerobe i have a browserify branch that
should have this addressed
https://github.com/alexgorbatchev/node-browserify


Reply to this email directly or view it on GitHubhttps://github.com/shtylman/node-browser-resolve/issues/20#issuecomment-17908025
.

I would like to present an argument that supporting a middle language that does compile to your language of choice that you make effort to support only makes your efforts more successful :)

I've been writing in ecma script since early 2000, but switched to coffee because i find it's easier to digest and write... but i still think in ecma, coffee is just a different outfit for it.

I would like to call for no coffee discrimination ;)

Contributor

andreypopp commented May 22, 2013

@shtylman is there any progress with this issue? I asked @substack on #browserify and he seems to be ready to merge corresponding pull request into module-deps and browserify after this one is merged

Owner

defunctzombie commented May 22, 2013

I have been travelling lately and not had a chance to review this yet. I will add the feature tho; I have given up fighting this. I personally think it is stupid but realize other people are doing it this way.

Collaborator

substack commented May 22, 2013

This seems like such a minor thing. I already do require('./file.js') sometimes because I like how explicit that is. Extensions are global and mutating globals gets really weird and uncomposable really fast.

Raynos commented May 22, 2013

I think require('./file") should only be used for folders

Contributor

andreypopp commented May 23, 2013

@substack @Raynos for me it's not a minor thing I use coffeescript and

  • if I require my .coffee files with extensions and use coffeeify transform — then after publishing my built js files on npm my imports stop working, because now the extension of my modules is .js
  • if I do separate build step before browserify then
    • I have to lay down all built files on filesystem which is sometimes a lot of burden
    • I have to provide a way for sourcemaps to work while with using coffeeify they work out of the box

Given that this change is already supported by node-resolve and is a single-line change, I think, it would make just a little tiny piece of harm to browserify in terms of complexity and at the same time this resolves a lot of burden for AltJS (CoffeeScript in my case) langs developers.

Raynos commented May 23, 2013

@andreypopp the fact that coffeescript does not compile require("x.coffee") to require("x.js) is clearly a bug.

Contributor

andreypopp commented May 23, 2013

@Raynos I do not think that it is really sane to allow a compiler to mangle string literals, I would consider this a flaw of CommonJS module system then, because it is uses string literals and not static identifiers for importing modules

Raynos commented May 23, 2013

Then you should just only have require("x.js") references and when you compile it into js there is no string munging.

Contributor

andreypopp commented May 23, 2013

Then I have to compile sources separately before running even in nodejs and give up CoffeeScript development tools (such a REPL)

Andrey Popp

On 23.05.2013, at 12:18, Raynos notifications@github.com wrote:

Then you should just only have require("x.js") references and when you compile it into js there is no string munging.


Reply to this email directly or view it on GitHub.

@ELLIOTTCABLE ELLIOTTCABLE added a commit to ELLIOTTCABLE/Paws.js that referenced this issue May 24, 2013

@ELLIOTTCABLE ELLIOTTCABLE (new meta) Coverage tooling! (And a shit-ton of other stuff.)
This is a fucking mess, too. In so many ways, not controllable by myself. Where to start ...

The `require()` problem™
========================
 - coffeeify (for browserify 2.x) *requires* that every `require()` statement explicitly end with
   the string ".coffee", to grab the file and bundle it into the browserify bundle
 - after instrumenting for coverage, the relevant (*instrumented*) files are `.js` files; thus, all
   of my `require('fuck-me.coffee')` statements will fail within the instrumented code; not to
   mention that the tests themselves also have to directly require the `.coffee` files
 - browserify will only work with `require()` statements for which the first argument is a direct
   string-literal in the source-code; so we can't work around this with the obvious
   complex-expression *in-place*

So, until the whole clusterfuck that is substack/coffeeify#1 is fixed (see also:
defunctzombie/node-browser-resolve#19, defunctzombie/node-browser-resolve#20), I've created a shim-file that
*wraps* the local `require` for each file in another function which mangles the name as necessary
based on the coverage environment variables, and then *replace* the local `require` function
in-scope with that wrapper.

Thus (phew), we can:

1. Satisfy browserify 2.x with ASTs of the exact form `require('<string literal>')` (hey, *it*
   doesn't know that the *value* of the `require` token has been re-defined!)
2. Satisfy coffeeify with string-literals ending in `.coffee`
3. Generate coverage for instrumented plain-JavaScript files that actually *resolve* all of their
   requires

Fuck Cake
=========
What the title says.

Node is great for a lot of things, but “rake”-style task management isn't one of them. I briefly
considered simply using Rake, even though I'm in a JavaScript project ... but then I decided to go
with the Node-y UNIX flow, and just resort to shell scripts. I'll be writing them generically (to
function in everything from dash, to bash, up to zsh); the remainder of migration from `Cakefile` to
`Scripts/*.sh` will happen in later commits. For now, though, I completely failed to reasonably
employ the available coverage tools through Node's `child_process` and similar, got fed up, and
immediately implemented both the `npm run-script coverage` and `npm run-script coveralls` as
shell-scripts, and just called out to *those* from the existing Cakefile.

Due to me being lazy in that way, we might have a hybrid Cake and shell-script build system for a
short while. Meh.

Travis-CI and Coveralls.io
==========================
There isn't any *official* support for Node (or JavaScript at all, for that matter) from Coveralls;
there's a third-party library, node-coveralls, but it's absolutely terrible. I've patched it and
pull-requested (see: nickmerwin/node-coveralls#6) to make it reasonably usable for my purposes here;
I've also had to modify some Travis configuration to work with all this. We'll see if it works when
I push this commit.
75fb580

@ELLIOTTCABLE ELLIOTTCABLE added a commit to ELLIOTTCABLE/Paws.js that referenced this issue May 24, 2013

@ELLIOTTCABLE ELLIOTTCABLE (new meta) Coverage tooling! (And a shit-ton of other stuff.)
This is a fucking mess, too. In so many ways, not controllable by myself. Where to start ...

The `require()` problem™
========================
 - coffeeify (for browserify 2.x) *requires* that every `require()` statement explicitly end with
   the string ".coffee", to grab the file and bundle it into the browserify bundle
 - after instrumenting for coverage, the relevant (*instrumented*) files are `.js` files; thus, all
   of my `require('fuck-me.coffee')` statements will fail within the instrumented code; not to
   mention that the tests themselves also have to directly require the `.coffee` files
 - browserify will only work with `require()` statements for which the first argument is a direct
   string-literal in the source-code; so we can't work around this with the obvious
   complex-expression *in-place*

So, until the whole clusterfuck that is substack/coffeeify#1 is fixed (see also:
defunctzombie/node-browser-resolve#19, defunctzombie/node-browser-resolve#20), I've created a shim-file that
*wraps* the local `require` for each file in another function which mangles the name as necessary
based on the coverage environment variables, and then *replace* the local `require` function
in-scope with that wrapper.

Thus (phew), we can:

1. Satisfy browserify 2.x with ASTs of the exact form `require('<string literal>')` (hey, *it*
   doesn't know that the *value* of the `require` token has been re-defined!)
2. Satisfy coffeeify with string-literals ending in `.coffee`
3. Generate coverage for instrumented plain-JavaScript files that actually *resolve* all of their
   requires

Fuck Cake
=========
What the title says.

Node is great for a lot of things, but “rake”-style task management isn't one of them. I briefly
considered simply using Rake, even though I'm in a JavaScript project ... but then I decided to go
with the Node-y UNIX flow, and just resort to shell scripts. I'll be writing them generically (to
function in everything from dash, to bash, up to zsh); the remainder of migration from `Cakefile` to
`Scripts/*.sh` will happen in later commits. For now, though, I completely failed to reasonably
employ the available coverage tools through Node's `child_process` and similar, got fed up, and
immediately implemented both the `npm run-script coverage` and `npm run-script coveralls` as
shell-scripts, and just called out to *those* from the existing Cakefile.

Due to me being lazy in that way, we might have a hybrid Cake and shell-script build system for a
short while. Meh.

Travis-CI and Coveralls.io
==========================
There isn't any *official* support for Node (or JavaScript at all, for that matter) from Coveralls;
there's a third-party library, node-coveralls, but it's absolutely terrible. I've patched it and
pull-requested (see: nickmerwin/node-coveralls#6) to make it reasonably usable for my purposes here;
I've also had to modify some Travis configuration to work with all this. We'll see if it works when
I push this commit.
84c75de

mjpizz referenced this issue in jnordberg/coffeeify Jun 7, 2013

Closed

Automatically add .coffee/.litcoffee extensions #1

Owner

defunctzombie commented Aug 3, 2013

I believe this issue was resolved a while back. If it is still a problem please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment