Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context's app reference #25

Closed
timkurvers opened this issue Jul 28, 2014 · 10 comments
Closed

Context's app reference #25

timkurvers opened this issue Jul 28, 2014 · 10 comments

Comments

@timkurvers
Copy link

var assert = require('assert');
var koa    = require('koa');
var mount  = require('koa-mount');

var other = koa();
other.use(function* () {
  assert(this.app === other);
});

var app = koa();
app.use(mount('/other', other));
app.listen(3000);

Had expected the above to work, but it raises. Should the app reference in the context refer to the main/base app or the currently handling app?

@jonathanong
Copy link
Member

koa and the official middleware do not consider apps self-contained. always consider this and .app to always be that of the "root" app.

i don't think this is going to change.

@timkurvers
Copy link
Author

I see. What is then the idea behind modular, distributable apps? This would essentially mean that an app augmented with functionality which is relied upon in its routes would cease to function once mounted.

@jonathanong
Copy link
Member

this isn't a philosophical issue, purely implementational. we're just not sure how to implement this correctly, concisely, and make everyone happy, so we didn't bother at all.

@timkurvers
Copy link
Author

Ah, I'm sorry I misunderstood. I did give the following a shot locally, which seems to work for the setup I had:

diff --git a/index.js b/index.js
index 553260e..60acccc 100644
--- a/index.js
+++ b/index.js
@@ -47,21 +47,26 @@ function mount(prefix, app) {

   return function *(upstream){
     var prev = this.path;
+    var prevApp = this.app;
     var newPath = match(prev);
     debug('mount %s %s -> %s', prefix, name, newPath);
     if (!newPath) return yield* upstream;

     this.mountPath = prefix;
     this.path = newPath;
+    this.app = app;
     debug('enter %s -> %s', prev, this.path);

     yield* downstream.call(this, function *(){
       this.path = prev;
+      this.app = prevApp;
       yield* upstream;
+      this.app = app;
       this.path = newPath;
     }.call(this));

     debug('leave %s -> %s', prev, this.path);
+    this.app = prevApp;
     this.path = prev;
   }

I'd gladly open a pull request if this is something to have in the main repo.

@jonathanong
Copy link
Member

the problem is that you'll run into other issues like passing settings through each app. you don't want 100% encapsulation, but you also don't want 0%. if you have an idea of exactly how encapsulation would work, that would be great!

@timkurvers
Copy link
Author

Hm, good point. Currently, a mounted app that relies on settings through the context would need its settings to be applied to the root application, right?

What kind of settings would one like to share?

@jonathanong
Copy link
Member

this is the problem. people want different settings to be passed based on their app.

@timkurvers
Copy link
Author

Wouldn't it then be more correct and flexible to have settings separated and allow developers themselves to configure how they see fit?

That would mean in certain circumstances having to configure certain settings multiple times (e.g. custom envs or proxy), but that would make the whole thing more portable.

@luin
Copy link

luin commented Jan 28, 2015

This issue could be solved by creating an new context instead of using the parent's context:

before:

yield* downstream.call(this, function *(){

after:

var context = app.creatContext(this.req, this.res);
yield* downstream.call(context, function *(){

So that this.app will always refer to the current app instead of the "root" app. However this change of course breaks the backwards compatibility because koa-mount currently mounts the middlewares of an app rather than the app itself.

related issue: #5.


@jonathanong The current API(app.use(mount(otherApp))) is a little misleading as it indicates otherApp will be the sub-app of app(related issue koajs/session#33), and leaving out the backwards compatibility, I think app.use(mount(otherApp.middleware)) may be better.

@naxmefy
Copy link

naxmefy commented Feb 2, 2016

really hard trap....

koa = require 'koa'
mount = require 'koa-mount'
error = require 'koa-error'

rootApp = koa()
subApp = koa()

rootApp.use error()

rootApp.context.rofl = "Root not Groot"
subApp.context.rofl = "Sub?"
subApp.use (next) ->
    this.body = this.app.context.rofl
    yield next
rootApp.use mount subApp

rootApp.listen process.env.PORT, process.env.IP, (err) =>
    throw err if err
    console.log "Server running on #{process.env.IP}:#{process.env.PORT}"

this prints Root not Groot

But what if i put (like koajs documentation) the db to my context?

Example from koajs.com

app.context

The recommended namespace to extend with information that's useful throughout the lifetime of your application, as opposed to a per request basis.

app.context.db = db();

the mounting of koa apps means - mounting an encapsulated application - every other meaning make no sense - like this example...

koa = require 'koa'
mount = require 'koa-mount'
error = require 'koa-error'

rootApp = koa()
rootApp.context.db = rootDB()
rootApp.use error()

blogApp = koa()
blogApp.context.db = blogDB()
blogApp.use (next) ->
    results = yield @app.context.db().posts.sort('-createdBy').limit(10).exec()
    @state.latestPosts = results
    yield next

rootApp.use mount '/blog', blogApp

rootApp.listen process.env.PORT, process.env.IP, (err) =>
    throw err if err
    console.log "Server running on #{process.env.IP}:#{process.env.PORT}"

what could be a solution?

  1. every mounting app is using his own context
  2. the parent app will be injected e.g. as this.app.parent or more better and like express would be an event hook app.on('mount', function(parent){
koa = require 'koa'
mount = require 'koa-mount'
error = require 'koa-error'
_ = require 'lodash'

rootApp = koa()
rootApp.context.db = rootDB()
rootApp.use error()

blogApp = koa()
blogApp.on 'mount', (parent) ->
  _.merge blogApp.context.config, parent.context.config
  blogApp.context.db = blogDB blogApp.context.config.blog
# ...

rootApp.use mount '/blog', blogApp

rootApp.listen process.env.PORT, process.env.IP, (err) =>
    throw err if err
    console.log "Server running on #{process.env.IP}:#{process.env.PORT}"

this.state should be also shared because it is ruhsing throw the middlewares...

This is no solution...

module.exports = (subApp) ->
  return (next) ->
    this.app.context.db = subApp.context.db
    yield next

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

No branches or pull requests

4 participants