Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Controller#reuse, Controller#share, Controller#retrieve #643

Open
YAndrew91 opened this issue Jul 20, 2013 · 9 comments
Open

Controller#reuse, Controller#share, Controller#retrieve #643

YAndrew91 opened this issue Jul 20, 2013 · 9 comments

Comments

@YAndrew91
Copy link
Contributor

I've already mentioned this issue in #537, but I still haven't an answer. I've also faced a couple of use-cases when it would be helpful to be able to retrieve stale compositions.

I'll describe two major use-cases of using the Composer for preserving smth between two requests.

  • Ordinary case from Chaplin examples for preserving views (not only them, of course): every request can make previous compositions non-stale to prevent their deletion right after this request ends its work.
  • Case when you want to pass some data to the next controller. For example, controller A has loaded some data from the server, and controller B needs the same data (we don't want to request them again).

So, there is an obvious issue in the 2nd use-case: we only want to retrieve the data, but not to mark them as non-stale (I don't want my controller C to be able to get them too). For comparison, now I first need to mark the composition as non-stale, which in itself depends on the result of the check method (a rain dance again, because decision will be made by previous check, not by the one we are passing now).

Any thoughts?

@mehcode
Copy link
Contributor

mehcode commented Jul 20, 2013

Case when you want to pass some data to the next controller. For example, controller A has loaded some data from the server, and controller B needs the same data (we don't want to request them again).

Doesn't something like...

# controllers/a
show: ->
  @compose 'data', -> # ...
  data = @compose 'data'
  # Use data

# controllers/b
show: ->
  @compose 'data', -> # ...
  data = @compose 'data'
  # Use data

I'm not understanding why controllers/b doesn't want to 'compose' the data. The idea is that if B is using the data, it should compose them so the data doesn't get disposed. Or am I missing something here?

@YAndrew91
Copy link
Contributor Author

Maybe it was't a very convincing example, I just don't want to make these data accessible from controller C requested after B, instead I want them to be destroyed before C starts.

Real problems appear when you want not only to access stale data, but also probably to recompose them in the same controller B, depending on the passed options and result of the check composition method.

Here is how I have to do it now in my controller (inside action or beforeAction):

var prevData;

// marking previous composition as non-stale
// to be able to access its data
this.compose(
    'data',
    {
        options: {
            accepted: true
        },
        compose: function () {}
    }
);

// get previous data for further usage
prevData = this.compose('data').data;

// trying to recompose the data
this.compose(
    'data',
    {
        options: {
            controllerType: this.controllerType
        },
        check: function (options) {
            return options.accepted === true || options.controllerType === this.options.controllerType;
        },
        compose: function () {
            // set composition data
            this.data = ...;
        }
    }
);

Here the composition will be recreated only if controllerType was changed.

And here is how I want to do it:

var prevComposition,
    prevData;

prevComposition = this.compose('data');

// get previous data for further usage
prevData = prevComposition ? prevComposition.data : null;

// trying to recompose the data
this.compose(
    'data',
    {
        options: {
            controllerType: this.controllerType
        },
        check: function (options) { // redundant if checking all the options fields for equality            
            return options.controllerType === this.options.controllerType;
        },
        compose: function () {
            // set composition data
            this.data = ...;
        }
    }
);

@knuton
Copy link
Contributor

knuton commented Jul 21, 2013

Maybe it was't a very convincing example, I just don't want to make these data accessible from controller C requested after B, instead I want them to be destroyed before C starts.

Well, this is a general problem with using the composer for data: It allows controllers to access data from within the context of other controllers. How are you certain that the sequence is A-B-C anyway? Whether or not you can retrieve stale compositions, the control of accessing the composed data from A is in X's hands, where X is whichever controller is active following A.

It sounds like you are essentially using composition for caching remote data. Maybe I am just ignorant of earlier discussions of the Composer, but shouldn't caching of remote data happen someplace else? I find an adapter/cache close to the data model in question to be much more natural. Obviously this doesn't offer the explicit hook for retaining data in between controllers, but I would consider caching of model data to be mere optional optimisation anyway.

@molily
Copy link
Member

molily commented Aug 3, 2013

@YAndrew91: Maybe it was't a very convincing example, I just don't want to make these data accessible from controller C requested after B, instead I want them to be destroyed before C starts.

Well, the composer isn’t a security model. All objects that are “composed” are public. As @knuton says, the next controller decides whether to “receive” the objects. The objects are disposed automatically if C doesn’t reuse it.

@compose() currently performs:

  1. check if the composition exists and the payload can be reused (check)
  2. create the objects if necessary (typically models/views)
  3. save it for reuse by the next controller (i.e. mark as non-stale)
  4. (return the composition)

I agree that these are actually three separate things you might want to do or do not depending on the use case.

  • Some controllers just receive context data but cannot recreate the context if none is given (gallery > image, article list > article etc., see Dispatcher doesn't save params for previous route #537)
  • Receiving and/or creating objects doesn’t mean you want to save them for reuse by the next controller. This is probably a subtle difference, because the objects will be disposed anyway if the next controller action doesn’t reuse it. But the order is a bit different. Currently they are disposed after the controller action is called. AFAIR there were cases where people need to dispose models and views strictly before the next action is called.

Maybe we can find an API to reflect these differences? Quick ideas:

attributes = id: 1
# Just retrieve (don’t set stale = false)
composition = @retrieve 'foo'
# Retrieve and check
composition = @retrieve 'foo', attributes
# Retrieve, check, create if necessary (don’t set stale = false)
composition = @reuse 'foo', MyModel, attributes
# Save for the next controller
composition = @share 'foo'
composition = @share 'foo', someModelOrView
# Retrieve, check, create, share (set stale = false)
@reuseAndShare 'foo', MyModel, attributes
# This is basically what @compose is now

@knuton: Maybe I am just ignorant of earlier discussions of the Composer, but shouldn't caching of remote data happen someplace else?

The Composer can be used to cache remote data for a short time (i.e. 2-3 screens, for example list view > item detail view > list view). For caching data for a longer time (e.g. the whole application lifetime), the Composer would be too cumbersome. Then it’s more reasonable to have a central cache that has its own retention and cleanup logic.

@YAndrew91
Copy link
Contributor Author

Great! Now I see that you understand the issue. For now, instead of using the Composer, I have a simple storage API (mixin) for retrieving/creating/preserving data. So, under the hood this storage uses another hash than the Composer. But I still would like to have such feature out of the box (and more handsomely done).

Receiving and/or creating objects doesn’t mean you want to save them for reuse by the next controller.

Receiving - yes, creating - maybe I'm missing smth but for what? When do we need to create a composition and not to share it? So, I don't understand when we need reuse instead of share.

AFAIR there were cases where people need to dispose models and views strictly before the next action is called.

Are you about your latest post in #495? It is a bit disappointing that such an issue hasn't been solved yet. I've described my idea in #453.

The Composer can be used to cache remote data for a short time (i.e. 2-3 screens, for example list view > item detail view > list view). For caching data for a longer time (e.g. the whole application lifetime), the Composer would be too cumbersome. Then it’s more reasonable to have a central cache that has its own retention and cleanup logic.

Totally agree, that is the reason I've opened this issue: for example, how to simply pass already loaded model data from a list view into a details view.

@artyomtrityak
Copy link

Also we should keep in mind that we also should be able to get composition after method execution in callback #740

@artyomtrityak
Copy link

@paulmillr @molily what will be broken if we will allow to return object in @compose even it has stale=true state? I think this will solve all issues isn't it?

Example:

class Screen extends Chaplin.Controller
  show: ->
    @compose 'details', Details

  _doSomething1: ->
    el = @compose 'details'


class Screen2 extends Chaplin.Controller
  show2: ->
    @compose 'details', Details

  _doSomething2: ->
    el = @compose 'details'

will use same shared object.
I think it will make Chaplin flow too complex with different methods like @reuse / @retrive etc

And if we will make stale=true cleanup only after routing this should be okay

@paulmillr
Copy link
Contributor

not sure about this. we should test the idea.

But still I think we should rename compose to reuse even if there will be only one method.

reuse is much more intuitive

@artyomtrityak
Copy link

renaming is not big deal.

What flow should we use to test? Make changes in reuse brunch?

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

No branches or pull requests

6 participants