8000 Event emit skipping by TimNZ · Pull Request #862 · feathersjs/feathers · GitHub
[go: up one dir, main page]

Skip to content

Event emit skipping#862

Closed
TimNZ wants to merge 1 commit intofeathersjs:masterfrom
TimNZ:skip-events
Closed

Event emit skipping#862
TimNZ wants to merge 1 commit intofeathersjs:masterfrom
TimNZ:skip-events

Conversation

@TimNZ
Copy link
Contributor
@TimNZ TimNZ commented Apr 28, 2018

Refer to discussion here:
feathersjs-ecosystem/feathers-sequelize#188

Want granular control over event emitting after service methods.

You'll likely want to tidy up my test which is hacky.
I'm not knowledgeable on how you test for something NOT happening.

@daffl
Copy link
Member
daffl commented Apr 28, 2018

Thank you for the PR @TimNZ. Skipping even emitting should however already be possible by returning feathers.SKIP in the last hook you want to run.

8000

@TimNZ
Copy link
Contributor Author
TimNZ commented Apr 28, 2018

I knew about SKIP but didn't test well enough if it could achieve what I wanted.

Have tested nested service calls and a simple hook and modifying context/params achieves desired flow.

Though if any other 'finally' hooks ever get added in the future this will break them.
That's why I prefer the explicit event skipping, doesn't depend on any particular hook ordering

@TimNZ
Copy link
Contributor Author
TimNZ commented Apr 28, 2018

If you decide not to have explicit event skipping support,
perhaps a note in the SKIP section and Events page docs that SKIP prevents event emitting as well.
https://docs.feathersjs.com/api/hooks.html#returning-feathersskip

My event logging module is separate from hooks handling, I like the modular micro approach you've taken and don't want to introduce dependencies.

@daffl
Copy link
Member
daffl commented May 23, 2018

Haven't forgotten about this. Just figuring out if and how this will fit into the next release.

@TimNZ
Copy link
Contributor Author
TimNZ commented May 25, 2018

@daffl Thanks.

No rush.

A context flag to skip events processing, decoupled from hook flow, still makes most sense to me.

@daffl
Copy link
Member
daffl commented Aug 13, 2018

Allright, so what I'd like to allow for this release would be adding a hook context.event which will contain the service event name that will be sent once the hook completes successfully. That way it would be possible to change the event that is emitted or prevent any events from being sent by setting context.event = null.

@TimNZ
Copy link
Contributor Author
TimNZ commented Aug 13, 2018

Does the job.

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.

2 participants

0