search

simov / express-admin

MySQL, MariaDB, PostgreSQL, SQLite admin for Node.js
MIT License
1.18k stars 223 forks source link

Trying to read undefined view.mainview in mainview.js #38

Closed philippwiddra closed 10 years ago

philippwiddra commented 10 years ago

I'm not sure if that is intended and the problem was on my side, but I had to fix this to get the admin starting.

If the view.mainview variable is not defined, !view.mainview evaluates to true so the boolean expression continues to test with !view.mainview.show, which of course throws an error because node can't call a value from undefined.

You can easiely rewrite that as (view.mainview && view.mainview.show), because in this case view.mainview evaluates to false if it's undefined, and so the second part is not even checked (because it's an AND).

But as it appears also the view variable can be undefined, so I had to use (view && view.mainview && view.mainview.show).

That way this boolean expression represents the exact opposite of the original one which made it easy to also just omit the continue; part.

Here the original output with errorstack. The error appears after logging in, thats why the /login route logged normally.

GET /login 200 11ms - 7.68kb
POST /login 302 268ms - 58b
TypeError: Cannot read property 'mainview' of undefined
    at exports.get (c:\Users\Phil\Documents\GitHub\express-admin\routes\mainview.js:15:18)
    at callbacks (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:164:37)
    at exports.restrict (c:\Users\Phil\Documents\GitHub\express-admin\routes\auth.js:20:34)
    at callbacks (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:164:37)
    at param (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:138:11)
    at pass (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:145:5)
    at Router._dispatch (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:173:5)
    at Object.router (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\router\index.js:33:10)
    at next (c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\node_modules\connect\lib\proto.js:193:15)

    at c:\Users\Phil\Documents\GitHub\express-admin\node_modules\express\lib\application.js:121:9
GET / 500 14ms - 1.1kb
simov commented 10 years ago

Yes you are right I have this fixed on master but not pushed yet. And not in npm for sure.

This part is for loading the custom views into the mainview - http://simov.github.io/express-admin-site/#customjson but since you can completely omit the app key which represents the custom view it get's undefined.

The really bad thing about this is not the bug but instead that the app don't have full test coverage yet.

I'm closing this to avoid conflicts with my fix and yes that type of things are really annoying ..

simov commented 10 years ago

@philippwiddra this was the fix https://github.com/simov/express-admin/commit/bde27397b3a93fab14831a85a24e01978313bedf