{"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":29,"created_at":"2015-09-15T16:49:16Z","updated_at":"2015-11-21T20:43:37Z","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/158679705","html_url":"https://github.com/balderdashy/sails/pull/3235#issuecomment-158679705","issue_url":"https://api.github.com/repos/balderdashy/sails/issues/3235","id":158679705,"user":{"login":"niallobrien","id":1525669,"avatar_url":"https://avatars.githubusercontent.com/u/1525669?v=3","gravatar_id":"","url":"https://api.github.com/users/niallobrien","html_url":"https://github.com/niallobrien","followers_url":"https://api.github.com/users/niallobrien/followers","following_url":"https://api.github.com/users/niallobrien/following{/other_user}","gists_url":"https://api.github.com/users/niallobrien/gists{/gist_id}","starred_url":"https://api.github.com/users/niallobrien/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/niallobrien/subscriptions","organizations_url":"https://api.github.com/users/niallobrien/orgs","repos_url":"https://api.github.com/users/niallobrien/repos","events_url":"https://api.github.com/users/niallobrien/events{/privacy}","received_events_url":"https://api.github.com/users/niallobrien/received_events","type":"User","site_admin":false},"created_at":"2015-11-21T20:43:37Z","updated_at":"2015-11-21T20:43:37Z","body":"Would like to see some more movement on this too. :)"}}