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

Closed
rolandoyahoo opened this Issue Dec 5, 2012 · 12 comments

Projects

None yet

5 participants

@rolandoyahoo

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.

Collaborator
caridy commented Dec 5, 2012

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

Member
drewfish commented Dec 5, 2012

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?

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.

Member
drewfish commented Dec 5, 2012

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.

Collaborator
caridy commented Dec 6, 2012

@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?

Member
drewfish commented Dec 6, 2012

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

@drewfish drewfish added a commit to drewfish/mojito that referenced this issue Dec 6, 2012
@drewfish drewfish fix issue #835 more careful about caching results of expandInstanceFo…
…rEnv()
54dcebe
Member
drewfish commented Dec 7, 2012

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.)

"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'

Member

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.

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$

Member

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.

@drewfish drewfish closed this Dec 11, 2012

Works fine, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment