YahooArchive / mojito

[archiving soon] Yahoo! Mojito Framework
BSD 3-Clause "New" or "Revised" License
1.57k stars 214 forks source link

mojito@0.5.x context config cache is not recognizing selectors #835

Closed rolandoyahoo closed 11 years ago

rolandoyahoo commented 11 years ago

We defined selectors and a different title for the site dimension:

{
    "settings": [ "site:mysite1" ],
    "selector": "mysite1",
    "specs": {
      "frame" : {
        "config" : {
          "title": "My Site 1 Title"
        }
      }
    }
},
{
    "settings": [ "site:mysite2" ],
    "selector": "mysite2",
    "specs": {
      "frame" : {
        "config" : {
          "title": "My Site 2 Title"
        }
      }
    }
},

When the first request is processed, mojito selects the correct title from the site in the context but subsequent requests to either one of the two sites returns the same title.

Reviewing mojito source, the problems appears to be in 'mojito/lib/app/addons/rs/select.js' [line 82]:

var store = this.get('host'),

The variable 'store' is not initialized properly so in line 97 the condition is always false:

if (part.selector && store.selectors[part.selector]) {

This results in having a wrong configuration cache key in 'mojito/lib/app/autoload/store.server.js' line 538 and all requests end up using the same config regardless of contexts and selectors.

caridy commented 11 years ago

@jenny this has to be reviewed asap after 0.5.0GA.

drewfish commented 11 years ago

Hi,

I believe what you are seeing is an intended optimization by mojito. The idea is that even if you define a selector in your applications.json, if you don't have files with that selector then there's no reason to customized the configuration (and thus cache key) based on that selector. By only creating the customizations that are actually used by the app greatly increases app start time.

Is there an underlying problem that you are experiencing? What odd/wrong behavior are you seeing that lead to you to investigate this?

rolandoyahoo commented 11 years ago

The most basic issue we see is that the page title is wrong when using HTMLFrameMojit. In our config "specs.frame" is an HTMLFrameMojit so we are trying to override "specs.frame.config.title" for each site we serve with this app. There are other config problems but we would have to go into our code detail to explain.

In response to "By only creating the customizations that are actually used by the app greatly increases app start time" I have to say we are actually using config for each site even if we don't have site specific files in our app. I think your assumption that not having files for a given selector should not mean that you can not have configuration for it.

drewfish commented 11 years ago

Yeah, it sounds like something is going wrong inside of mojito. Could you email me your application repo (perhaps via yahoo mail, since I know you work at yahoo)?

The config that the resource store is generating (per selector, as appropriate) deals with the YUI loader configuration. Other configs, such as read by ycb, should still work normally regardless of which selectors you have defined. So, I suspect the bug isn't the one you suggest but is another one in a different place in mojito.

caridy commented 11 years ago

@rolandoyahoo in your aspects, it looks like you don't have base or type for "frame" definition, is that code an psuedo-code? or you really don't have such thing?

drewfish commented 11 years ago

I created a simple app that demonstrates the problem with the title: https://github.com/drewfish/mojito-issue835

drewfish commented 11 years ago

Hi @rolandoyahoo,

If possible, could you try using the HEAD of develop to see if that fixes your problem? (I suspect it'll fix at least some of your troubles, but want to make sure that there aren't other issues as well.)

You can get the lastest by putting this in your package.json: "mojito": "git://github.com/yahoo/mojito.git#develop" (Note, this causes "npm install" to do a git clone, so it takes a while.)

davesbatyahoo commented 11 years ago

"npm install" is dying immediately on:

TypeError: Cannot read property 'latest' of undefined at next (/usr/local/lib/node_modules/npm/lib/cache.js:524:35)

"ynpm install --registry [url]" finishes installing but dies when starting mojito in:

mojito-shaker/addons/rs/shaker.server.js:41 this.meta = this.rs.config.readConfigJSON TypeError: Object undefined[config] has no method 'readConfigJSON'

drewfish commented 11 years ago

It looks like you're using an older version of mojito-shaker. The version that works with the latest mojito is mojito-shaker@2.0.37pr5.

nottoseethesun commented 11 years ago

Earlier I had tried pr5, and then and now get this error pasted below. These things fail so often that kept it at pr4 for now and did not have time to report it. However if it is a potential blocker on this, then let's see what we can do to fix this - thanks! - >

vpn-pool-10-72-116-11:app cbalz$ pwd /Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app vpn-pool-10-72-116-11:app cbalz$ node_modules/mojito-shaker/bin/mojito-shake --debug 5 --run [SHAKER] - Initializing ShakerCore

/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito-shaker/addons/rs/shaker.server.js:45 this.meta = this.rs.config.readConfigSimple(libpath.join(this. ^ TypeError: Object undefined[config] has no method 'readConfigSimple' at RSAddonShaker.YUI.add.Y.extend.initializer (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito-shaker/addons/rs/shaker.server.js:45:44) at RSAddonShaker.YUI.add.v._initHierarchy (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-core/base-core-min.js:7:2892) at RSAddonShaker.YUI.add.v._baseInit (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-core/base-core-min.js:7:930) at RSAddonShaker.YUI.add.l._defInitFn (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-base/base-base-min.js:7:1303) at YUI.add.o.fireComplex (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/event-custom-complex/event-custom-complex-min.js:7:1752) at YUI.add.e.CustomEvent.fire (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/event-custom-base/event-custom-base-min.js:7:4346) at RSAddonShaker.YUI.add.E.fire (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/event-custom-base/event-custom-base-min.js:7:11039) at RSAddonShaker.YUI.add.l.init (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-base/base-base-min.js:7:842) at RSAddonShaker.YUI.add.v._initBase (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-core/base-core-min.js:7:791) at RSAddonShaker.YUI.add.l._initBase (/Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app/node_modules/mojito/node_modules/yui/base-base/base-base-min.js:7:550) vpn-pool-10-72-116-11:app cbalz$ npm ls salcap@0.0.1 /Users/cbalz/projects/y_salcap/desktop/salcap-desktop/app

vpn-pool-10-72-116-11:app cbalz$ npm ls | grep mojito ├─┬ mojito@0.5.0pr5 ├─┬ mojito-shaker@2.0.37pr5 ├── y_mojito@0.1.2 vpn-pool-10-72-116-11:app cbalz$

drewfish commented 11 years ago

I just tried a fresh checkout of the salcap repo, and git://github.com/yahoo/mojito.git#develop works, so I'm closing this as fixed. It'll get released as part of 0.5.1.

Please re-open if you are still having trouble with this.

nottoseethesun commented 11 years ago

Works fine, thanks!