Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

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

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

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

rolandoyahoo opened this issue Dec 5, 2012 · 12 comments
Labels

Comments

@rolandoyahoo
Copy link

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
Copy link
Contributor

caridy commented Dec 5, 2012

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

@drewfish
Copy link
Contributor

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?

@rolandoyahoo
Copy link
Author

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
Copy link
Contributor

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.

@caridy
Copy link
Contributor

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?

@drewfish
Copy link
Contributor

drewfish commented Dec 6, 2012

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

drewfish added a commit that referenced this issue Dec 7, 2012
fix issue #835 more careful about caching results of expandInstanceForEnv
@drewfish
Copy link
Contributor

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

@davesbatyahoo
Copy link

"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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link

Works fine, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants