[v4] Composition engine #404
Conversation
…4fire/core into bonkalol/composition-engine-v4
…position-engine-v4
| {cacheKey} = r.ctx; | ||
|
|
||
| if (cacheKey != null && !r.destroyed) { | ||
| boundedRequests.set(cacheKey, r); |
There was a problem hiding this comment.
@kobezzza вот тут не уверен, но лучше ничего не придумал
Мне нужно собирать request которые сделал движок чтобы при вызове engine.dropCache или engine.destroy вызывать эти же методы у всех запросов которые были созданы движком. Соответственно чтобы это сделать я держу карту [cacheKey: request] в которой сохраняю созданные запросы.
Может возникнуть коллизия и тогда старый request будет заменен свежесозданным, это ок? Не создам ли я учетек памяти тут?
There was a problem hiding this comment.
Тут проще завести локальный асинк и в момент создания каждого запроса просто регать воркер, а потом в финальном деструкторе сделать claerAll
| error; | ||
|
|
||
| try { | ||
| result = await request('', {engine}).data; |
There was a problem hiding this comment.
Сейчас сигнатурой request не предусмотрена возможность не передавать url запроса, что в целом то логично.
Вообще движок композиций для request модуля не совсем понятно зачем может быть нужен, основные бенефиты получает провайдер с движком (понятно что там тоже request используется, но тем не менее), а request мы и так можем композировать как хотим.
| const | ||
| requestFilter = provider.requestFilter?.(options, params); | ||
|
|
||
| if (requestFilter === false) { |
There was a problem hiding this comment.
Может просто завернуть в Promise.resolve и тогда получится простая логика: есть фильтр и нет фильтра
| }, reject); | ||
|
|
||
| function boundRequest<T extends Provider | RequestResponseObject | RequestPromise>(request: T): T { | ||
| if (request instanceof Provider) { |
There was a problem hiding this comment.
Тут надо ввести контракт, что если есть методы dropCache или destroy, а не смотреть на инстанс реквеста
| const resultPromise = (() => { | ||
| if (opts?.aggregateErrors) { | ||
| return Promise.allSettled(promises) | ||
| .then((results) => { |
There was a problem hiding this comment.
Вынеси колбек как отдельно функцию
|
|
||
| r.destroy = (...args) => { | ||
| boundedRequests.delete(cacheKey); | ||
| forkedDestroy(...args); |
| const response = new Response(result, { | ||
| parent: options.parent, | ||
| important: options.important, | ||
| responseType: 'json', |
There was a problem hiding this comment.
А почему json , а не object?
| context = params.opts.meta.provider ?? params.ctx, | ||
| providerParams = context instanceof Provider ? context.params : undefined; | ||
|
|
||
| if (context instanceof Provider && !providersWithEngine.has(context)) { |
There was a problem hiding this comment.
Почему проверка именно на провайдера? тут мне кажется надо просто смотреть утиной типизацией наличие методов
| boundedRequests = new Map<string, RequestResponseObject>(); | ||
|
|
||
| const | ||
| providersWithEngine = new WeakSet(); |
| }); | ||
|
|
||
| engine.dropCache = (recursive) => { | ||
| boundedRequests.forEach((request) => request.dropCache(recursive)); |
There was a problem hiding this comment.
Мне не нравиться, что ты явно разделяешь сущности - запрос и провайдер. Это должно быть без разницы. Можно воспользроваться асинк: он умеет работать как с воркерами, так и с абртабл промисами и т.д.
| ```typescript | ||
| const r = request({engine: compositionEngine([ | ||
| { | ||
| request: (opts, params, {boundRequest}) => boundRequest(request('http://locahost:5555/json/1')), |
There was a problem hiding this comment.
А нельзя избавиться от потребности явно вызывать bound?
|
|
||
| } | ||
|
|
||
| const val1 = await boundRequest(new Provider1()).get().data; |
| @@ -35,6 +35,8 @@ import type { RequestEngine, RequestOptions } from 'core/request/interface'; | |||
| import { availableParams, deniedProviderParams } from 'core/request/engines/provider/const'; | |||
| import type { AvailableOptions, MethodsMapping, Meta } from 'core/request/engines/provider/interface'; | |||
|
|
|||
| export * from 'core/request/engines/composition'; | |||
| @@ -155,6 +155,11 @@ export interface RequestResponseObject<D = unknown> { | |||
| * Destroys the request context | |||
| */ | |||
| destroy(): void; | |||
|
|
|||
No description provided.