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

Deprecate model hooks and events #473

Merged
merged 2 commits into from
Feb 27, 2015

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Feb 26, 2015

Deprecate the following Model hooks:

  • beforeValidate
  • afterValidate
  • beforeCreate
  • afterCreate
  • beforeSave
  • afterSave
  • beforeUpdate
  • afterUpdate
  • beforeDestroy
  • afterDestroy

Deprecate the following Model events:

  • changed
  • deletedAll
  • deleted

Closes strongloop/loopback#617.

/to @raymondfeng @ritch please review

@bajtos
Copy link
Member Author

bajtos commented Feb 27, 2015

I added one more commit to fix depd in browser. We may want to extract "lib/browser.depd.js" into a standalone module in the future, or perhaps contribute the simpler version to depd. I don't have enough time for that right now.

@raymondfeng @ritch ping, is this good to be merged?

Miroslav Bajtoš added 2 commits February 27, 2015 16:58
List of deprecated hooks:

 - beforeValidate
 - afterValidate
 - beforeCreate
 - afterCreate
 - beforeSave
 - afterSave
 - beforeUpdate
 - afterUpdate
 - beforeDestroy
 - afterDestroy

Also add a lightweight browser version of "depd", because the "depd"
does not support browser and it is not trivial to fix that.
This commits adds a lightweight implementation of depd's "deprecate"
function.
List of deprecated events:

 - changed
 - deleted
 - deletedAll
@bajtos bajtos force-pushed the feature/deprecate-model-hooks-and-events branch from edc52f1 to 37d7721 Compare February 27, 2015 16:03
@bajtos
Copy link
Member Author

bajtos commented Feb 27, 2015

I made one more change. Initially, event deprecation messages were printed at the time when the deprecated event was emitted. That's a wrong approach, because depd will print a stack showing where the event was emitted, which is not helpful in identifying the place which is listening for this deprecated event. I have reworked the implementation to print the deprecation message from Model.on() and cleaned up the commit history along the way.

The PR is ready for review and merge.

@ritch
Copy link
Contributor

ritch commented Feb 27, 2015

LGTM

@bajtos
Copy link
Member Author

bajtos commented Feb 27, 2015

For posterity, the CI build passed on 5 out of 6 machines.

screen shot 2015-02-27 at 19 34 24

bajtos added a commit that referenced this pull request Feb 27, 2015
…-and-events

Deprecate model hooks and events
@bajtos bajtos merged commit 17a999b into master Feb 27, 2015
@bajtos bajtos removed the #review label Feb 27, 2015
@bajtos bajtos deleted the feature/deprecate-model-hooks-and-events branch February 27, 2015 18:37
@bajtos
Copy link
Member Author

bajtos commented Feb 27, 2015

@crandmck could you please update the docs and reword the warning to a deprecation note? http://docs.strongloop.com/display/LB/Model+hooks

I am not sure if the we have the model events (Model.on('changed'), deleted, deletedAll) documented anywhere. If we do, then please update that page as well.

@crandmck
Copy link
Contributor

Added deprecation notice. I removed

However, afterInitialize is still useful.
Should that stay in?

Also removed the following events from http://docs.strongloop.com/display/LB/Events and http://docs.strongloop.com/display/LB/Basic+model+object:

  • changed
  • deleted
  • deletedAll

@bajtos Please confirm that is correct, and I will also remove from http://apidocs.strongloop.com/loopback/#model.

@bajtos
Copy link
Member Author

bajtos commented Feb 27, 2015

@crandmck Thank you.

However, afterInitialize is still useful.

Should that stay in?

Yes please, afterInitialize is not deprecated and still useful. It is also different from the rest of the model hooks because it's synchronous (i.e. there is no callback).

Also removed the following events from http://docs.strongloop.com/display/LB/Events

Can that page mention that there are Operation hooks that should (and can) be used instead of the deprecated events?

Also perhaps the page http://docs.strongloop.com/display/LB/Basic+model+object should link to "Operation hooks" instead of "Model hooks"?

@crandmck
Copy link
Contributor

OK, added note about afterInitialize, and note about using operation hooks instead of the deprecated events.

@bajtos bajtos mentioned this pull request Jun 6, 2016
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

Successfully merging this pull request may close these issues.

Model.upsert does not trigger changed event
3 participants