Locals are not being overridden when passed via req.options.locals

[megakoresh]

I have encountered a very weird bug. I have a policy that runs before every request generating a quick navigation list for the site's content. The nav list is basically a nested categorized list of the format

{
  "Category1": [{
      "Item1Name": "Name",
      "Item1ID": ID
    },{
      "Item2Name": "Name",
      "Item2ID": ID
    }
  ],
 "Category2": []
}

And so on...
The policy creates the list under the req.options.locals.navlist. The problem is that when I change a category of something, it will still show up in it's old category as well as the new one, until I re-lift the app. If I log the object from a view it shows that after re-arranging the items old entry is being kept in memory anyway and the new correct list simply gets added on top. If I log the list from the policy itself, there are no problems, the list is correct.

Policy code is like so:

module.exports = function(req, res, next){
  if(true){ //supposed to be a user ID here later
    Asset.find({author: 'DefaultUser', sort: 'createdAt DESC'}).exec(function(err, assets){
      if(!err && assets.length>0){
        sails.log.verbose('Building navlist');
        var fullNames = utilService.fillFullNames(assets);
        req.options.locals = {
          navlist: _.groupBy(fullNames, 'categoryFullName')
        };
      }
      next();
    });
  }
}

Then in my controller I have it like

return res.view('homepage', {navlist: req.options.locals.navlist || []})

Is this a bug? I would think that if I pass a new variable by the same name the old should be overridden. I also have some global locals (thats a funny phrase) for all views under config.views.locals setting.

[sailsbot]

Hey @megakoresh, would you mind adding information about your Sails, Node, and NPM versions to this issue? In this case, it looks like the knowledge of which view engine (default EJS, jade, etc) you're using would also be helpful. Thanks!

[megakoresh]

No, sailsbot, I do not think it would be helpful, since the issue has to do with passing variables around, and switching the view engine does not do anything. But have it anyway:
Sails: v0.12-rc5
NPM: v3.3.6
Node: 5.0.0
View engine: Jade

[mikermcneil]

No, sailsbot, I do not think it would be helpful, since the issue has to do with passing variables around, and switching the view engine does not do anything

Hey @megakoresh- different view engines interpret locals differently. For example, EJS allows you to provide configuration for the view engine itself by passing them amongst the locals (believe it or not x_x). Anyways, just worth pointing out that in some cases, this can make a huge difference. Would you post your version of Jade in this issue just for future reference? Part of the reason for collecting as much information about dependency versions as possible is to provide a trail of clearly documented breadcrumbs if we find ourselves hunting down semi-related issues in the future.

My guess is this could be an issue stemming from

sails/lib/hooks/views/res.view.js

Line 153 in 61a72d6

_.merge(locals, sails.config.views.locals, _.defaults);

. While merge is supposed to function right-to-left, we did recently update the lodash version in Sails as part of 0.12, so it sounds like you may have discovered a related problem. I think the thing to do here is to replace the merge with a shallow call to _.defaults()-- and although the deep merge was never officially documented behavior, I'll document the move to a shallow merge as a breaking change in 0.12 just to be safe. Took a step in that direction w/ 5861f65, but we also need a test for this in core.

Before going too much further down this road though, would you verify something for me in your local project?

• run your app w/ sails console
• perform the necessary requests to get some properties lingering in req.options.locals
• enter sails.config.views.locals in the REPL and look at the output (it should be all messed up too)

Thanks again for your help!

[megakoresh]

As a matter of fact it isn't messed up. When I enter that in console it doesn't return the navlist at all - only the globally available locals (and before you ask - yes, I did try removing them to no effect). Logging req.options.locals.navlist right after policy reconstructs it shows the correct list. Logging the navlist from a view shows the messed up list with new and old entries. Logging req.options.locals from a controller also shows the messed up list without the globally available locals. It seems so very confusing...

I updated my sails installation to the latest version from github with your change, but it didn't fix the issue. By the way if you are wondering what utilService.sortByProperty does, it's just an alias for lodash groupBy function. And it does properly work, seeing as logging the object straight from the policy shows the correct one.

My Jade version is 1.11.0. Now I need to sleep, it's almost 1am... Will be back at this tomorrow.

UPDATE: Despite my head feeling like an atomic bomb ready explode I stuck around am glad I did: I found the problem, though not the logic behind it.

When I returned the results from the database to the req.options.locals as they originally were (without refactoring them with _.groupBy) and moved the re-factoring to the view instead, it seems to properly react to changes now. Why was this a problem, considering that the policy logging returned a normal list is still a mystery though, so not closing yet.

[mikermcneil]

@megakoresh hmm, perhaps a global leak? If sails.config.views.locals wasn't getting messed up, it doesn't seem likely to be coming from Sails core -- of course you never know. I'd suggest creating a new sails app real quick and attempting to reproduce the error there. If you can get it reproduced that way, please let me know and I'll dive in.

(Leaving this open to remind us to update the docs for sails.config.views.locals)

[sgress454]

Final postmortem on this--looks like it was actually due to an issue introduced in 166e772, with the way that req.options were being merged with the options specified during the initial route binding. Fixed in dfa2765.