YahooArchive / mojito

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

mojito gets confused if multiple mojitos are found #812

Closed jlecomte closed 11 years ago

jlecomte commented 11 years ago

My mojito app depends on mojito (duh!) and on a package (let's call it "X") which itself lists mojito as a dev dependency (so I can run "mojito test app" to test that package) If I type "npm link" in package X src dir, it will install mojito under its node_modules/ folder. Now, if I npm link that package to my app, the resource store will find multiple mojitos while walking the file system at startup. It seems like it sometimes gets confused in that particular situation. I think the resource store should skip any folder named mojito except for the one that the app immediately depends on.

caridy commented 11 years ago

We had a discussion about this problem few days ago, in a different context, but it boils down to the same problem: whether or not we should flatten a multi-level structure, or just analyze the first level of modules.

My vote is for not supporting mojito entities at a deeper level, only the first level, which makes a lot of sense since putting and application together means aggregating and flatting a group of mojito entities with a custom configuration at the app level.

That doesn't means an npm package can't require another npm pkg, it can, but it can't not rely on the mojito definition for that pkg.

jlecomte commented 11 years ago

Caridy: let's not hijack the issue I reported (which I think is a pretty simple fix) with something which, although somewhat relevant, is an entirely different discussion. Let's chat offline because I very strongly disagree with your comment.

caridy commented 11 years ago

This should be covered in 0.5.3, where mojito keeps track of the version and pkg information to avoid walking similar pkgs. @drewfish can you confirm and close?

drewfish commented 11 years ago

Yep, this was fixed in mojito@0.5.3. Since mojito walks NPM packages breadth-first (all first-level packages are walked before second-level, those before 3rd level, etc), the "shallower" version of mojito, likely the direct dependency of the app, will be used.

jlecomte commented 11 years ago

I am still seeing this issue with mojito 0.5.4 when linking a package that requires mojito 0.5.x.

drewfish commented 11 years ago

Hi @jlecomte, could you give more details? The output of npm ls would be a good start.

jlecomte commented 11 years ago

$ ynpm ls search-apps-news@0.0.1 /git-data/apps/search-apps-news ├─┬ jsdom@0.2.1 │ ├── cssom@0.2.5 │ ├── htmlparser@1.7.6 │ └─┬ request@2.14.0 │ ├─┬ form-data@0.0.7 │ │ ├── async@0.1.22 │ │ └─┬ combined-stream@0.0.4 │ │ └── delayed-stream@0.0.5 │ └── mime@1.2.9 ├── mockery@1.4.0 ├─┬ mojito@0.5.4 │ ├── colors@0.6.0-1 │ ├─┬ express@2.5.10 │ │ ├─┬ connect@1.9.2 │ │ │ └── formidable@1.0.12 │ │ ├── mkdirp@0.3.0 │ │ └── qs@0.4.2 │ ├─┬ glob@3.1.21 │ │ ├── graceful-fs@1.2.0 │ │ ├── inherits@1.0.0 │ │ └─┬ minimatch@0.2.11 │ │ ├── lru-cache@2.2.2 │ │ └── sigmund@1.0.0 │ ├─┬ js-yaml@1.0.2 │ │ └─┬ argparse@0.1.12 │ │ ├── underscore@1.4.4 │ │ └── underscore.string@2.3.1 │ ├─┬ jslint@0.1.9 │ │ └─┬ nopt@1.0.10 │ │ └── abbrev@1.0.4 │ ├── mime@1.2.4 │ ├── mkdirp@0.3.5 │ ├── request@2.9.202 │ ├─┬ rimraf@2.1.4 │ │ └── graceful-fs@1.2.0 │ ├── semver@1.0.14 │ ├── wrench@1.3.9 │ ├── yui@3.7.3 │ ├─┬ yuidocjs@0.3.38 │ │ ├── graceful-fs@1.2.0 │ │ ├─┬ minimatch@0.1.5 │ │ │ └── lru-cache@1.0.6 │ │ ├── node-markdown@0.1.1 │ │ ├─┬ rimraf@2.0.3 │ │ │ └── graceful-fs@1.1.14 │ │ └─┬ yui@3.9.0pr1 │ │ └─┬ request@2.12.0 │ │ ├─┬ form-data@0.0.3 │ │ │ ├── async@0.1.9 │ │ │ └─┬ combined-stream@0.0.3 │ │ │ └── delayed-stream@0.0.5 │ │ └── mime@1.2.7 │ ├── yuitest@0.7.8 │ └── yuitest-coverage@0.0.6 ├─┬ mojito-shaker@3.0.7 │ ├── async@0.1.22 │ ├── gear@0.8.3 │ ├─┬ gear-lib@0.8.7 │ │ ├── csslint@0.9.10 │ │ ├─┬ handlebars@1.0.10 │ │ │ └─┬ optimist@0.3.5 │ │ │ └── wordwrap@0.0.2 │ │ ├─┬ jslint@0.1.9 │ │ │ └─┬ nopt@1.0.10 │ │ │ └── abbrev@1.0.4 │ │ ├── knox@0.0.11 │ │ ├── less@1.2.2 │ │ └── uglify-js@1.2.6 │ ├─┬ glob@3.1.21 │ │ ├── graceful-fs@1.2.0 │ │ ├── inherits@1.0.0 │ │ └─┬ minimatch@0.2.11 │ │ ├── lru-cache@2.2.2 │ │ └── sigmund@1.0.0 │ ├── mime@1.2.9 │ ├── mkdirp@0.3.5 │ ├── semver@1.0.14 │ └─┬ yui@3.7.3 │ └── request@2.9.202 ├─┬ mojito-shaker-mobstor@1.1.1 │ ├── mime@1.2.9 │ └── mobstor@1.1.4 ├─┬ search-apps-base@0.1.33 │ └── async@0.1.22 ├── y_mojito@0.1.2 ├─┬ yahoo-mojits-search-bread-crumb@0.0.15-11 │ ├── yahoo-utils-string-truncate@0.0.11 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-core@0.4.0-120 -> /git-data/common/mojits/yahoo-mojits-search-core/src │ ├── mockery@1.4.0 │ ├─┬ mojito@0.5.5 │ │ ├── colors@0.6.0-1 │ │ ├─┬ express@2.5.10 │ │ │ ├─┬ connect@1.9.2 │ │ │ │ └── formidable@1.0.12 │ │ │ ├── mkdirp@0.3.0 │ │ │ └── qs@0.4.2 │ │ ├─┬ glob@3.1.21 │ │ │ ├── graceful-fs@1.2.0 │ │ │ ├── inherits@1.0.0 │ │ │ └─┬ minimatch@0.2.11 │ │ │ ├── lru-cache@2.2.2 │ │ │ └── sigmund@1.0.0 │ │ ├─┬ js-yaml@1.0.2 │ │ │ └─┬ argparse@0.1.12 │ │ │ ├── underscore@1.4.4 │ │ │ └── underscore.string@2.3.1 │ │ ├─┬ jslint@0.1.9 │ │ │ └─┬ nopt@1.0.10 │ │ │ └── abbrev@1.0.4 │ │ ├── mime@1.2.4 │ │ ├── mkdirp@0.3.5 │ │ ├── request@2.9.202 │ │ ├─┬ rimraf@2.1.4 │ │ │ └── graceful-fs@1.2.0 │ │ ├── semver@1.0.14 │ │ ├── wrench@1.3.9 │ │ ├── ycb@1.0.4 │ │ ├─┬ yui@3.8.1 │ │ │ └─┬ request@2.12.0 │ │ │ ├─┬ form-data@0.0.3 │ │ │ │ ├── async@0.1.9 │ │ │ │ └─┬ combined-stream@0.0.3 │ │ │ │ └── delayed-stream@0.0.5 │ │ │ └── mime@1.2.7 │ │ ├─┬ yuidocjs@0.3.38 │ │ │ ├── graceful-fs@1.2.0 │ │ │ ├─┬ minimatch@0.1.5 │ │ │ │ └── lru-cache@1.0.6 │ │ │ ├── node-markdown@0.1.1 │ │ │ ├─┬ rimraf@2.0.3 │ │ │ │ └── graceful-fs@1.1.14 │ │ │ └─┬ yui@3.9.0pr1 │ │ │ └─┬ request@2.12.0 │ │ │ ├─┬ form-data@0.0.3 │ │ │ │ ├── async@0.1.9 │ │ │ │ └─┬ combined-stream@0.0.3 │ │ │ │ └── delayed-stream@0.0.5 │ │ │ └── mime@1.2.7 │ │ ├── yuitest@0.7.8 │ │ └── yuitest-coverage@0.0.6 │ ├── y_mojito@0.1.2 │ ├── yahoo-utils-globals@0.0.9 │ ├── yahoo-utils-query-highlight@0.0.14 │ ├── yahoo-utils-relative-time@0.0.20 │ ├── yahoo-utils-scheduler@0.3.5 │ ├── yahoo-utils-search-environment@0.0.15 │ ├─┬ yahoo-utils-search-preferences@0.2.5 │ │ └── yahoo-utils-sonora@0.1.7 │ ├── yahoo-utils-string-truncate@0.0.11 │ └── yahoo-utils-url@0.1.20 ├── yahoo-mojits-search-debug@0.0.7-10 ├─┬ yahoo-mojits-search-image-dd@0.0.15-22 │ ├─┬ yahoo-utils-query-highlight@0.0.14 │ │ └── yahoo-utils-globals@0.0.9 │ ├── yahoo-utils-string-truncate@0.0.11 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-media-search-result@0.0.17-12 │ ├── yahoo-utils-relative-time@0.0.20 │ ├── yahoo-utils-string-truncate@0.0.11 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-related-concepts@0.0.12-14 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-sort-by-options@0.0.13-15 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-time-filter@0.0.13-2 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-video-dd@0.0.18-15 │ ├─┬ yahoo-utils-query-highlight@0.0.14 │ │ └── yahoo-utils-globals@0.0.9 │ ├── yahoo-utils-string-truncate@0.0.11 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-video-duration-filter@0.0.12-12 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├─┬ yahoo-mojits-search-webresult-dd@0.0.15-8 │ ├─┬ yahoo-utils-query-highlight@0.0.14 │ │ └── yahoo-utils-globals@0.0.9 │ └─┬ yahoo-utils-url@0.1.20 │ └── yahoo-utils-globals@0.0.9 ├── yahoo-utils-csf@0.2.16 ├─┬ yahoo-utils-debug@0.0.16 │ └── yahoo-utils-globals@0.0.9 ├── yahoo-utils-jscheck@0.0.3 ├── yahoo-utils-lazyload@0.0.3 ├─┬ yahoo-utils-lpmproxy@0.2.0 │ ├── colors@0.6.0-1 │ ├─┬ handlebars@1.0.10 │ │ ├─┬ optimist@0.3.5 │ │ │ └── wordwrap@0.0.2 │ │ └── uglify-js@1.2.6 │ └─┬ yui@3.7.3 │ └── request@2.9.202 ├── yahoo-utils-markup-test@0.0.16 ├─┬ yahoo-utils-search-environment@0.0.15 │ └── yahoo-utils-globals@0.0.9 ├── yahoo-utils-ult@0.0.22 └── ycb@1.0.4

jlecomte commented 11 years ago

yahoo-mojits-search-core is a linked package that contains another mojito instance with a different version. Here is the error I get:

[ERROR][Cosmo:Registry] Could not load application i1362524287 due to exception TypeError: Object # has no method 'makeStaticHandlerDetails' at process (/git-data/apps/search-apps-news/node_modules/yahoo-mojits-search-core/node_modules/mojito/lib/app/addons/rs/yui.js:699:68) at RSAddonYUI.YUI.add.Y.extend.resolveResourceVersions (/git-data/apps/search-apps-news/node_modules/yahoo-mojits-search-core/node_modules/mojito/lib/app/addons/rs/yui.js:708:17) at ResourceStore.YUI.add.e.rbind (/git-data/apps/search-apps-news/node_modules/mojito/node_modules/yui/oop/oop-min.js:7:2149) at YUI.add.i.Method.exec (/git-data/apps/search-apps-news/node_modules/mojito/node_modules/yui/event-custom-base/event-custom-base-min.js:7:1350) at ResourceStore.YUI.add.i._inject.o.(anonymous function).r.(anonymous function) as resolveResourceVersions at ResourceStore.YUI.add.Y.extend.preload (/git-data/apps/search-apps-news/node_modules/mojito/lib/app/autoload/store.server.js:407:18) at Object.Store.createStore (/git-data/apps/search-apps-news/node_modules/mojito/lib/store.js:91:15) at new MojitoServer (/git-data/apps/search-apps-news/node_modules/mojito/lib/mojito.js:69:29) at Object.Mojito.createServer (/git-data/apps/search-apps-news/node_modules/mojito/lib/mojito.js:685:12) at Object. (/git-data/apps/search-apps-news/server.js:125:29)

drewfish commented 11 years ago

OK, thanks, I'll dig into it.

Does yahoo-mojits-search-core need to be linked?

jlecomte commented 11 years ago

That mojit is linked during development. It does not have to be, but it is useful to be able to do so.

drewfish commented 11 years ago

I'm able to reproduce, in all of the following: ./dev, ./node_modules/.bin/mojito-shaker shake, and ./node_modules/.bin/mojito start.

I don't think the link is related to the problem at all.

drewfish commented 11 years ago

Hmmm... this is a tricky problem. You have two versions of mojito installed in your app: mojito@0.5.4 is shallow in the dependency tree, and mojito@0.5.5 is deep in the dependency tree.

We can only use one version of mojito. Which should we use? One the one hand, we should use the "shallowest" since it represents a more specific/tighter dependency. This is especially true if it is a direct dependency of the app. (The app is saying "use this version of mojito".) However, in your case, you have a version of mojito deeper in the dependency tree with has a higher version. Presumably some other NPM module needs this higher version of mojito and that's why they specified it explicitly.

Complicating this particular situation is the fact that the resource store API changed between 0.5.4 and 0.5.5.

Additionally, mojito-shaker appears to be using whatever version of mojito that NPM returns (via require()), which in your case is the shallower version of mojito. This prevents the yahoo-mojits-search-core package from getting version 0.5.5.

drewfish commented 11 years ago

After talking with Caridy, the consensus is that we should be using the shallow version of mojito. This is what is used today. So, mojito just needs to skip other versions of mojito.

drewfish commented 11 years ago

This should be fixed by PR #1023 (which was just merged). It'll be in the next release of mojito (after 0.5.5).