{"action":"created","issue":{"url":"https://api.github.com/repos/balderdashy/sails/issues/3235","labels_url":"https://api.github.com/repos/balderdashy/sails/issues/3235/labels{/name}","comments_url":"https://api.github.com/repos/balderdashy/sails/issues/3235/comments","events_url":"https://api.github.com/repos/balderdashy/sails/issues/3235/events","html_url":"https://github.com/balderdashy/sails/pull/3235","id":106598419,"number":3235,"title":"Update to Express 4","user":{"login":"Josebaseba","id":1160665,"avatar_url":"https://avatars.githubusercontent.com/u/1160665?v=3","gravatar_id":"","url":"https://api.github.com/users/Josebaseba","html_url":"https://github.com/Josebaseba","followers_url":"https://api.github.com/users/Josebaseba/followers","following_url":"https://api.github.com/users/Josebaseba/following{/other_user}","gists_url":"https://api.github.com/users/Josebaseba/gists{/gist_id}","starred_url":"https://api.github.com/users/Josebaseba/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Josebaseba/subscriptions","organizations_url":"https://api.github.com/users/Josebaseba/orgs","repos_url":"https://api.github.com/users/Josebaseba/repos","events_url":"https://api.github.com/users/Josebaseba/events{/privacy}","received_events_url":"https://api.github.com/users/Josebaseba/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":12,"created_at":"2015-09-15T16:49:16Z","updated_at":"2015-09-22T15:43:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/balderdashy/sails/pulls/3235","html_url":"https://github.com/balderdashy/sails/pull/3235","diff_url":"https://github.com/balderdashy/sails/pull/3235.diff","patch_url":"https://github.com/balderdashy/sails/pull/3235.patch"},"body":"Hi guys!\r\n\r\nAs you know Sails works with Express3 which is deprecated since August. I think that we've to move to v4 as soon as posible, so I've tried to help in this huge work.\r\n\r\nI think that I have the 99% of the job done, but I need help doing more intensive testing to make sure that all work fine.\r\n\r\nCheck the [migration guide](http://expressjs.com/es/guide/migrating-4.html)\r\n\r\nThe changes that I've been forced to made:\r\n\r\n**HOOKS**\r\n\r\n**Blueprints**\r\n- Express 4 deprecated the req.param method in favor of req.params['param'] and req.query\r\n\r\n**Controllers**\r\n- Indentation changes\r\n\r\n**Cors**\r\n- A small fix to make sure that ```routeCorsConfig``` is always an object\r\n\r\n**CRSF**\r\n- Removed connect dependency for `CSURF`, the official *crsf* express package\r\n\r\n**HTTP**\r\n- That's one of the hooks with biggest changes, the router now [works different](http://expressjs.com/4x/api.html#router), removing `app.use(app.router);` that.\r\n- Unbind works a little bit different, now is a little bit more complex but it's not a big deal.\r\n- The 404 and 500 errors are added to the router in a different place, we are force to it because of the new router. I add those two middlewares at the absolutely end. When sails triggers the ready event.\r\n- Now the session works with express-session package\r\n- The default middleware order changed removing 404 and 500 from here and adding csrf (in case that the csrf hook is activated)\r\n- Compression works with express `compression` package\r\n- **FAVICON** now is served with `serve-favicon`, and we are forced to add a default favicon in `hooks/http/assets/favicon.ico`, that's because the `serve-favicon` is forced to had a favicon, if it couldn't find one the app crashes.\r\n\r\n**Session**\r\n- Again, removed connect session in favor of `express-session`.\r\n- Added a couple of defaults attributes\r\n\r\n**User Hooks**\r\n- Indentation stuff\r\n\r\n**Views**\r\n- That was hard, express4 doesn't work with layouts, so I've to add a layout support with a couple of functions inside the res.view method.\r\n\r\nThat in the hooks, now:\r\n\r\n**ROUTER**\r\n- The same happens here, but the changes are really minimum.\r\n- In the default handlers we use the new sintaxis to send responses .status().send()\r\n- New way to unbind routes\r\n- `cookie-parser` & `express-session` packages added\r\n\r\n**Package.json**\r\n- Added the latest express: `4.13.3`\r\n- `cookie-parser`\r\n- `csurf`\r\n- `express-session`\r\n\r\n**TEST**\r\n*I've to change some test to make them work with the new Express*\r\n\r\n- The router test is updated, because of the router changes. Now to check if the routes are binded or unbinded we have to look inside the `.stack` object.\r\n- In the sample app controller the way to check `route.key` and `route.method` is updated to `route.stack[0].method` and `route.stack[0].keys`. *I realized that the keys now works differently and we don't be able to get them always, I'm not sure why express does that*\r\n\r\n**CORS and CRSF** \r\n- In the CORS test, in the lines `64, 83 and 111` I can't get a 200 response code and I can't find why. That 3 tests are the only ones that don't work. I need some help there! And I can't get the `HTTP METHODS` complete list at `92` line. Those lines are commented right now.\r\n- Also, I've changed the `sampleApp/views/viewTest/crsf.ejs` file because it was wrong, we need to check if _crsf is null not if it is undefined, so I changed to the method: `_.isNull()`. Now it works like it should.\r\n\r\n- In the initializeHooks file, I removed supertest in favor of sails.request method, but the tests logic still as always and the same thing happen with the `router.bind.test.js` file.\r\n\r\n- In the router test file, I've added the `.stack` method\r\n- Unbind file with indentation changes\r\n\r\nMore or less that's all, I know that it's hard to make that kind of pull request work. But I would like to have some feedback about what do you guys have in mind for the express4 update.\r\n\r\nThanks!!"},"comment":{"url":"https://api.github.com/repos/balderdashy/sails/issues/comments/142328380","html_url":"https://github.com/balderdashy/sails/pull/3235#issuecomment-142328380","issue_url":"https://api.github.com/repos/balderdashy/sails/issues/3235","id":142328380,"user":{"login":"sgress454","id":553428,"avatar_url":"https://avatars.githubusercontent.com/u/553428?v=3","gravatar_id":"","url":"https://api.github.com/users/sgress454","html_url":"https://github.com/sgress454","followers_url":"https://api.github.com/users/sgress454/followers","following_url":"https://api.github.com/users/sgress454/following{/other_user}","gists_url":"https://api.github.com/users/sgress454/gists{/gist_id}","starred_url":"https://api.github.com/users/sgress454/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sgress454/subscriptions","organizations_url":"https://api.github.com/users/sgress454/orgs","repos_url":"https://api.github.com/users/sgress454/repos","events_url":"https://api.github.com/users/sgress454/events{/privacy}","received_events_url":"https://api.github.com/users/sgress454/received_events","type":"User","site_admin":false},"created_at":"2015-09-22T15:43:22Z","updated_at":"2015-09-22T15:43:22Z","body":"> Why keep Express3 inside the private router if we can replace it for pillarjs router?\r\n\r\nIt's really just been a case of, \"if it ain't broke, don't fix it\".  Especially since we were still tied to Express for other things.  We're not opposed to changing the private router, ever, it just needs to be done super carefully.\r\n\r\n> Are you going to manage all these changes internally?\r\n\r\nI frankly doubt it will happen at all without community help.  The basis of a standalone HTTP hook are here in this PR already.  The sticky wicket is the reliance of the `views` hook on Express's `res.render` functionality..."}}