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

Inheriting Remote Hooks #702

Closed
wtfribley opened this issue Oct 25, 2014 · 12 comments
Closed

Inheriting Remote Hooks #702

wtfribley opened this issue Oct 25, 2014 · 12 comments

Comments

@wtfribley
Copy link

I have two models:

/common/models/parent.json

{
  "name": "Parent",
  "base": "PersistedModel",
  "properties": {},
  "validations": [],
  "relations": {},
  "acls": [],
  "methods": []
}

/common/models/parent.js

module.exports = function(Parent) {
  Parent.afterRemote('**', function(ctx, result, next) {
    console.log('after remote ', ctx.methodString);
    next();
  });
};

/common/models/child.json

{
  "name": "Child",
  "base": "Parent",
  "properties": {},
  "validations": [],
  "relations": {},
  "acls": [],
  "methods": []
}

I would expect that the Parent's afterRemote hook is invoked when any of the Child's remote methods are called -- i.e. that hooks are inherited. Should I not expect this? Is there an alternate way for models to share hooks?

@violet-day
Copy link

I have the same question like u.But it's beforeCreate.Child will inherit Parent hook.

@bajtos
Copy link
Member

bajtos commented Oct 27, 2014

I am afraid this is a limitation of the current implementation of remoting hooks:

ModelCtor.afterRemote = function (name, fn) {
  // ...
      var className = self.modelName;
      remotes.after(className + '.' + name, function (ctx, next) {
        fn(ctx, ctx.result, next);
      });
  // ...

A workaround is to configure the hooks inside the setup function, which is called whenever the model is extended.

/common/models/parent.js

module.exports = function(Parent) {
  Parent.setup = function() {
    Parent.base.setup.apply(this, arguments);

    this.afterRemote('**', function(ctx, result, next) {
      console.log('after remote ', ctx.methodString);
      next();
    });
  };

  Parent.setup();
};

@bajtos bajtos changed the title Inheriting Model and Remote Hooks Inheriting Remote Hooks May 7, 2015
@rus0000
Copy link

rus0000 commented May 30, 2015

Seems same with a remoteMethod as well. It only works if defined inside overloaded setup method in parent model.

@bajtos Some questions about example:
/common/models/parent.js
1.
Parent.base.apply(this, arguments); - doesn't work
Parent.base.setup.apply(this, arguments); - works

Why need to call Parent.setup(); if it is

called whenever the model is extended

@bajtos
Copy link
Member

bajtos commented Jun 1, 2015

ad 1) Good catch, I have updated my comment above. It was a typo on my side.

ad 2) IIRC, loopback calls only one setup method. Consider the following inheritance hierarchy: Model -> PersistedModel -> YourBase -> YourModel. When YourModel is created, loopback will call YourBase.setup and it's up to that method to call PersistedModel.setup.

@rus0000
Copy link

rus0000 commented Jun 1, 2015

  1. Sorry for annoyance. It's still not clear to me.
    In our example we have inheritance chain: Model -> PersistedModel -> Parent -> Child

Loopback calls Child.setup, which is probably missing, therefore Parent.setup is invoked by searching prototype chain, as we have defined it with Parent.setup = function() { ... .
In Parent.setup we call base class method with Parent.base.setup.apply(this, arguments); which is defined in PersistedModel.setup. That's fine.

But why there is a call of Parent.setup(); at the end of an example. Why to call itself immediately after definition?

@bajtos
Copy link
Member

bajtos commented Jun 1, 2015

But why there is a call of Parent.setup(); at the end of an example. Why to call itself immediately after definition?

AFAIK, LoopBack does not call setup function when a new model is defined. In other words, if your Child model provides a setup function, it is called only when somebody extends (inherits from) your Child, but not when the Child is defined as a standalone model. See #550.

Sorry for annoyance. It's still not clear to me.

No worries, I am happy to explain. Is there anything else to explain in order to make this matter more clear?

@rus0000
Copy link

rus0000 commented Jun 1, 2015

OK. We provide setup function for Parent. It is called by Loopback when the Parent is extended. But why need to call it explicitly in a moment of definition?

@bajtos
Copy link
Member

bajtos commented Jun 1, 2015

But why need to call it explicitly in a moment of definition?

Because if you do not call it explicitly, then it won't be called for your leaf (non-base) model. You can verify this yourself by adding the following Car model to your app:

// common/models/car.json
{
  "name": "Car"
}

// common/models/car.js
module.exports = function(Car) {
  Car.setup = function() {
    console.log('setup');
  };
};

// server/model-config.json
{
  "Car": {
    "dataSource": "db" 
  }
}

@bajtos bajtos added this to the #Epic: LoopBack 3.0 milestone Nov 26, 2015
@bajtos
Copy link
Member

bajtos commented May 24, 2016

/cc @davidcheung you were investigating model inheritance recently. I think the conclusion was that we should prefer users to put stuff-to-be-inherited in the setup() method instead of auto(magic) inheritance. What do you think about this particular use case?

@bajtos bajtos removed this from the #Epic: LoopBack 3.0 milestone May 24, 2016
@davidcheung
Copy link
Contributor

@bajtos yeah that was the conclusion (more info)
I was testing it the other day and couldn't reach my use case, I think the very last child will need to recursively call their parents constructor, but from the base -> child direction, but that isnt the behavior
I need to do a little more investigation into this issue but yes I think we shouldn't do both inherit AND build.

@bajtos
Copy link
Member

bajtos commented Nov 12, 2019

LoopBack version 3 is in LTS, we won't be making breaking changes like this one.

@bajtos bajtos closed this as completed Nov 12, 2019
@itsdun1
Copy link

itsdun1 commented Mar 3, 2020

does setup() inheritence also works with afterRemoteError()

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

No branches or pull requests

8 participants