First steps to decouple routes from app.js #97
Conversation
This is just a first step, which will result in a cleaner app.js entry point, which just instructs other, more streamlined, objects when they've to do the "real" work. Routes are a big part of it. This commit for now only extracts the routes to a Routes object, which currently is as big as the app.js routes part and will be minimized in future commits.
These services are used to register a mobile device to the currently logged in user account. By this, the methods are decoupled from the routes.
|
This is a quite big PR, however, if I can help in any way to better understand what I did or to answer questions, feel free to ask and comment :] |
|
@FlorianSW, |
|
@FlorianSW you even have tests!!!!! Awesome! I'll give this a shot today! |
|
Hi I'm getting this error when I first startup node, I have not looked at what's causing the error yet.
|
|
Hi @FlorianSW , I was checking this PR, since I was making up my mind to bring in Google Home Actions. Short remarks: Thanks a lot.. |
|
Hi @marziman, just a quick note: I already read your comment, but had no time to look over the code, yet. It's totally possible, that my IDE tricked me, so I'll recheck all usages and correct them where necessary. Please give me some time for it, as I'm not sure, when in the next days I find the time to do that :/ Sorry for the delay! Best, |
|
@FlorianSW, |
|
@marziman Ok, yeah, this was an error, however, not from my IDE it seems, I just got confused while merging these changes in, iirc. However, I fixed it and merged the latest changes in :) |
|
Sorry about being away for so long! I will try and test again today. |
|
Throwing this error now 2017-07-16T14:01:19.172Z - error: uncaughtException: Cannot read property 'send' of undefined date=Sun Jul 16 2017 14:01:19 GMT+0000 (UTC), pid=12792, uid=1001, gid=1001, cwd=/home/openhabcloud/openhabcloud, execPath=/home/openhabcloud/.nvm/versions/node/v7.5.0/bin/node, version=v7.5.0, argv=[/home/openhabcloud/.nvm/versions/node/v7.5.0/bin/node, /home/openhabcloud/openhabcloud/app.js], rss=135163904, heapTotal=105340928, heapUsed=81062968, external=19355751, loadavg=[0.0615234375, 0.01513671875, 0.00390625], uptime=16394343, trace=[column=8, file=/home/openhabcloud/openhabcloud/routes/api.js, function=Object.exports.notificationssettingsget, line=36, method=exports.notificationssettingsget, native=false, column=127, file=/home/openhabcloud/openhabcloud/routes/index.js, function=Routes.setupAppRoutes, line=240, method=setupAppRoutes, native=false, column=10, file=/home/openhabcloud/openhabcloud/routes/index.js, function=Routes.setupRoutes, line=57, method=setupRoutes, native=false, column=4, file=/home/openhabcloud/openhabcloud/app.js, function=, line=317, method=null, native=false, column=32, file=module.js, function=Module._compile, line=571, method=_compile, native=false, column=10, file=module.js, function=Object.Module._extensions..js, line=580, method=Module._extensions..js, native=false, column=32, file=module.js, function=Module.load, line=488, method=load, native=false, column=12, file=module.js, function=tryModuleLoad, line=447, method=null, native=false, column=3, file=module.js, function=Function.Module._load, line=439, method=Module._load, native=false, column=10, file=module.js, function=Module.runMain, line=605, method=runMain, native=false, column=7, file=bootstrap_node.js, function=run, line=418, method=null, native=false, column=9, file=bootstrap_node.js, function=startup, line=139, method=null, native=false, column=3, file=bootstrap_node.js, function=null, line=533, method=null, native=false], stack=[TypeError: Cannot read property 'send' of undefined, at Object.exports.notificationssettingsget (/home/openhabcloud/openhabcloud/routes/api.js:36:8), at Routes.setupAppRoutes (/home/openhabcloud/openhabcloud/routes/index.js:240:127), at Routes.setupRoutes (/home/openhabcloud/openhabcloud/routes/index.js:57:10), at Object. (/home/openhabcloud/openhabcloud/app.js:317:4), at Module._compile (module.js:571:32), at Object.Module._extensions..js (module.js:580:10), at Module.load (module.js:488:32), at tryModuleLoad (module.js:447:12), at Function.Module._load (module.js:439:3), at Module.runMain (module.js:605:10), at run (bootstrap_node.js:418:7), at startup (bootstrap_node.js:139:9), at bootstrap_node.js:533:3] |
|
My fault (seems to be a merge error from my side :(). We really need some smoke tests to catch these kind of problems :/ I probably take a look into this in the next time and figure out how we can do that :) However, I'll fix that now. |
The function should not be called, but should be passed as a callback app.all().
|
That looks good, I just went through a quick smoke test routes seem correct.
Agreed, it would be nice to have something like travis ci running tests on every PR and merge. |
|
I opened #113, so that we don't forget that :) |
At the moment, all routes are setup and handled inside the main entry point app.js. This works great from a functional point of view, but makes the file big, hard to read and understand and therefore hard to maintain. The whole app.js file is also already a big container of code, which makes it even harder to write and review changes in it. A more separated objeect/module structure for handlers, initializers and callbacks would make it easier to maintain the code and bring new features into it.
This is a first step of achieving this (hopefully), by, more or less:
The next steps in the next time will be to move on this way. However, I stopped here for now, as it's already quite a big PR (and I would understand, if no-one want to review it, however, I hope someone will take the time to do :]).