Stream is returned when require('browjadify')('template.jade') is called from the CLI

[microadam]

Doing the following works on the CLI

require('browjadify/compile')('template.jade')

However this will break in the browser

[bengourley]

The ideal scenario

I am going to explain the reasons that this issue surfaced, and the possible solutions…

There are three distinct environments in which this module is designed to be used:

1. in the browser

Code that is processed with browserify and runs in the browser.

var compileJade = require('browjadify')
  , myTemplate = compileJade(__dirname + '/template.jade')

2. in the build

Code that runs the transform either in a build system or from the command line

browserify . -o bundle.js -t browjadify

or

var browserify = require('browserify')

var b = browserify('main.js')
b.transform('browjadify')

b.bundle().pipe(fs.createWriteStream('bundle.js'))

3. in node

Code that is meant for the browser (see point number 1) but which is running in node because it is being unit tested without the overhead of a browser

Why it doesn't work

I tried to get it so that in all 3 of these situations, using browjadify is as simple as require('browjadify').

require('browjadify') resolves to browjadify/transform in node (due to main field in package.json, and browser.js in the browser (due to browser field in package.json).

This works in situation 1 and 2, but unfortunately not 3. Situation 3 needs browjadify/compile but it gets browjadify/transform – hence why @microadam has reported getting a stream rather than a template function.

Potential solutions

I can see two possible solutions, neither of which I am particularly fond of:

1

To apply the transform, the user must require('browjadify/transform') in JS or browjadify -t browjadify/transform from the command line.

Browser/node code can continue to require('browjadify') and have it resolve correctly to browjadify/browser and browjadify/compile in each context.

I don't like this because browserify transform users expect to just apply the transform by module name, and not have to deal with reach into the module for a specific file. However, a project will contain an instance of this code exactly once for each bundle that it creates – whereas within the browser code, require('browjadify') will be called many times. This makes the refactor required for projects updating to this change will have the least effect.

2

Browser/node code must require('browjadify/compile') and have it resolve correctly to browjadify/browser and browjadify/compile in each context.

To apply the transform, the can continue to require('browjadify') in JS or browjadify -t browjadify from the command line.

This literally makes more sense – require('browjadify') gives you the browserify transform, as advertised. However, it means that every piece of code using a template would have to update from require('browjadify') to require('browjadify/compile'). This means more typing, and a larger refactor to any projects that want to stay up to date.

Opinions or alternative solutions please

Are either of these solutions ok, and I'm just being unnecessarily picky? Or is there a better solution that I can't see? Help!

Thanks for reading if you got this far.

[bengourley]

3

Stay as is and check for the presence of an environment variable in browjadify/transform to cater for situation 3.

[bengourley]

Or it could rely on process.title === 'browser'?

https://github.com/defunctzombie/node-process/blob/master/browser.js#L40

Then in your tests you would just need to set process.title = 'browser'. Presumably that's possible and not read only?

Thoughts @microadam, @domharrington?

[domharrington]

process.title could definitely be useful for this case. Is that documented anywhere? Is there a potential for it to break?

[bengourley]

http://nodejs.org/api/process.html#process_process_title

[bengourley]

I would have thought process.platform would be better, and that something within browserify should set process.platform = 'browser'. But it appears not to.

[domharrington]

That definitely sounds like something that should be done. Do you think that process.title is a reasonable fix? And adding that to each test where that is needed? Will setting that in one test pollute the process for everyone? Could that have any repercussions?

[bengourley]

I think it's my preferred fix. It will be required to add process.title = 'browser' to every test process (not every test) as process is global. I can't think of any likely repercussions. Maybe if a CLI that sets its own title was being tested in the same process as some browser code there would be a conflict. But that is pretty unlikely (and bad anyway) no?

[bengourley]

Hmm, on OSX the character limit for a process title is 4! So browser is truncated to brow. I think maybe we should use an environment variable. Then you can set it from outside or within process.

[domharrington]

Why is it so small? Env variable could work...

[bengourley]

After much deliberation, I'm going with solution 2. It makes the most sense. Having to type an extra '/compile' once per file that requires templates is not a problem.

[domharrington]

I think it makes more sense that for a browserify transform, require('browjadify') returns the transform. Upgrading existing projects could be a bit of a pain, but nothing more than one find/replace.

[bengourley]

Yeah. Before I posted that I came up with a 'better' idea which I typed out here… I was going to suggest that another module was created (something like 'cf-compile-jade') which is the thing you require from the browser code. This makes even more sense, but then the refactor is still a find and replace, and then another dependency of your project! Hence why I deleted it and went for this option.

[bengourley]

This isn't fixed. The browser field only works internally and for the 'main' package. It does not work for reaching into a module's file structure like we are doing here: require('browjadify/compile'). The result of this is that the entire jade compiler is currently being included on the frontend unnecessarily.

I have opened an issue here: substack/node-browserify#804 but I don't know if will get implemented. In the mean time browjadify needs fixing immediately.

[bengourley]

Fixed properly in 173d77627a69cfae17b247e4e7de01274833ac20