Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Express 4 #127

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

johngeorgewright
Copy link
Contributor

Now that Express 4 no longer excepts objects as middleware we have to use callbacks instead.

To keep asset-rack working in express 4, with some kind of fallback capabilities for express 3, I'm proposing to specify the Rack#handle method when adding the rack/asset as middleware. I've also upgraded CoffeeScript, so I could easily bind the class's method to any instance
of it with the new fat-arrow syntax.

Another problem is the express.Response#locals property is no longer a function. Therefore I've added the assets' "locals" property directly to
the express instance.

Now that Express 4 no longer excepts objects as middleware we have to
use callbacks instead.

To keep asset-rack working in express 4, with some kind of fallback
capabilities for express 3, I'm proposing to specify the `Rack#handle`
method when adding the rack/asset as middleware. I've also upgraded
CoffeeScript, so I could easily bind the class's method to any instance
of it with the new fat-arrow syntax.

Another problem is the `express.Response#locals` property is no longer a
function. Therefore I've added the assets' "locals" property directly to
the express instance.
@ryanbillingsley
Copy link

@johngeorgewright I am getting an error when trying to use this with express 4.1.1

TypeError: Property 'locals' of object #<ServerResponse> is not a function
    at Layer.exports.Rack.Rack.handle (/Users/ryanbillingsley/Dev/covert-deer/node_modules/asset-rack/compiled/rack.js:79:16)

@ryanbillingsley
Copy link

Never mind, looks like it is choking on coffee-script not being compiled now

@johngeorgewright
Copy link
Contributor Author

Maybe I'm using a newer version of CoffeeScript?

@ryanbillingsley
Copy link

The problem is in the switch.js

try {
    module.exports = require('./compiled');
} catch(error) {
    //require('./node_modules/coffee-script');
    require('coffee-script');
    module.exports = require('./lib');
}

require is failing to find the module './lib'

It could be because I am loading the module in through git and not through a module, not sure.

@ryanbillingsley
Copy link

I think this would be the problem jashkenas/coffeescript#3279

Coffee-script introduced a breaking change to how requiring coffee-script works.

@johngeorgewright
Copy link
Contributor Author

Good catch.

@martindale
Copy link

This is failing for me on both the latest version of Express (4.3.2) and the version listed here (4.1.1), with a request that never seems to get handled (server just hangs on all requests...)

@johngeorgewright
Copy link
Contributor Author

Hey @martindale, are you giving the #handle method to express#use?

https://github.com/johngeorgewright/asset-rack/blob/express-4/README.md#getting-started

I think that's what was happening to me until I made this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants