search

edmellum / browserijade

A Browserify middleware that pre-compiles Jade templates on the server and uses the light-weight Jade runtime made for the browser to render them on the client.
21 stars 6 forks source link

glob > 2.0.9 breaks browserijade #1

Closed kiernanmcgowan closed 12 years ago

kiernanmcgowan commented 12 years ago

It appears that newer versions of glob will break browserijade, as a result of the large changes between glob 2.x and 3.x. Thus installing browserijade via npm will result in code that is unusable unless a glob 2.x is already installed.

To fix this I just specified a specific version of glob.

Also, the example in the ReadMe was not accurate with how the code needed to be used, so I updated that example as well

Cheers

edmellum commented 12 years ago

Wow, I wasn't aware anyone was actually using this. Thanks for the pull request!

I'm pretty sure I've got a version of Browserijade that hasn't been pushed to Github yet. So I'll take a look at what I've got locally first then we'll see about what we need from your pull request.

I'd also like to apologize for the star in the dependencies, I feel stupid enough when it just bites me. Knowing it's bitten someone else is just plain sad.

kiernanmcgowan commented 12 years ago

Cool, no worries about the star dependencies, sometimes its just a pain to worry about what version you actually need.

Also, I would recommend adding browserify to the dependencies list as well, but I don't know what your specific use case is.

edmellum commented 12 years ago

Browserijade needs to be used with Browserify to be useful, but doesn't need Browserify for its internal operations. If Browserify was a dependency it would be downloaded or linked into Browserijade's node_modules directory causing unneeded overhead.

edmellum commented 12 years ago

Alright, I've committed all the stuff I had locally and pushed it up and I believe it fixes everything mentioned here. The API actually breaks a bit, but it's for the better as it conforms to the readme now and is sane.

The only thing I'm wondering about is line 16 in your readme bundle.require('browserijade');. I haven't needed to do this and it shouldn't be needed if it is. Could you test with the newest version and see if you really need it?

kiernanmcgowan commented 12 years ago

Ok, with the new version of the code I am able to use the example from your readme, but I am unable to successfully run the command browserijade(__dirname + "/views/browser").

For some the glob code ends up breaking whenever the command assert(this instanceof Glob). What version of node do you happen to be running?

kiernanmcgowan commented 12 years ago

K looks like my issue was part of an old version of node. Thanks for updating your package!