patronum icon indicating copy to clipboard operation
patronum copied to clipboard

implement `abort` method

Open AlexandrHoroshih opened this issue 4 years ago • 3 comments

closes #154

Description

This method creates abortable effects, which is mostly useful for cancellable api calls and all kinds controllable timeouts and intervals (e.g. for granular control over animations)

Checklist for a new method

  • [x] Create directory for new method in root in param-case
  • [x] Place code to {method-name}/index.js in CommonJS export in camelCase named export
  • [x] Add types to {method-name}/index.d.ts
  • [x] Add tests to {method-name}/{method-name}.test.ts
  • [x] Add fork tests to {method-name}/{method-name}.fork.test.ts
  • [x] Add type tests to test-typings/{method-name}.ts
    • Use // @ts-expect-error to mark expected type error
    • import { expectType } from 'tsd' to check expected return type
  • [x] Add documentation in {method-name}/reade.md
    • Add header Patronum/MethodName
    • Add section with overloads, if have
    • Add Motivation, Formulae, Arguments and Return sections for each overload
    • Add useful examples in Example section for each overload
  • [x] Add section to README.md in root
    • Add method to table of contents - [MethodName](#methodname)
    • Add section ## [MethodName](/method-name 'Documentation')
    • With summary and simple example

AlexandrHoroshih avatar Oct 01 '21 18:10 AlexandrHoroshih

Updated abort implementation to new patronum approach to operator implementations

Important questions:

  • Abortable Effect now throws special kind of Error, if effect was manually aborted. It would be very conventional to have special helper for failData to filter AbortedError. Discussion: https://github.com/effector/patronum/pull/168#discussion_r720620810
  • Abortable Effect now checks call keys and signal key by ===. But it might make sense to add special values for common cases (like ALL to abort all in-flight calls) or maybe even custom key-filter function to support multiple ways to group effect calls at once. Discussion: https://github.com/effector/patronum/pull/168#discussion_r720620635

Feel free to join these discussions

AlexandrHoroshih avatar Oct 11 '21 14:10 AlexandrHoroshih

Why don't create abort property on createEffect?

Lonli-Lokli avatar Jan 17 '22 09:01 Lonli-Lokli

@Lonli-Lokli

const hideFx = createEffect(() => {
  document.body.classList.add('hidden');
})

hideFx.abort();

What the result of .abort() call we need to do inside effect?

sergeysova avatar Jul 11 '22 10:07 sergeysova

Closed because the idea needs serious work

Good parts.

  • The onCancel hooks inside the effects body to register cleaning callbacks
  • Call key idea, to cancel only certain calls based on logical rules

Bad parts. getKey doesn't work if there are no parameters to get the call key/identifier, it's not clear what to do if there are multiple calls with the same call key. The idea of using the "call group" label to abort all calls from that group sounds good, but it doesn't work if we actually want both options - cancelling the entire group (e.g., absolutely all calls to this effect) in one case, and only certain calls in another. This suggests that one call should have multiple keys, or maybe there should be some generic abortKeyFilter for effect - but that's not very convenient, since all the logic options for choosing specific calls to cancel are merged into either one keyFilter function or a key generator.

AlexandrHoroshih avatar Jan 06 '23 18:01 AlexandrHoroshih