[BUG] Cached calls to `load()` still result in new Promise creation

Aperta
  • Qualcosa non va?
#377 1 commento 1 reazione 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
javascript, node.js
Ambito
backend-api-design

Direzione di ricerca

Inizia da src/index.js intorno alla riga 92 e confronta il percorso della cache con l’affermazione della documentazione relativa alla creazione delle promise. Esamina il commit 06c403bd72abef401dd7619c8ae992f03ed6b7d3 e determina se i caricamenti ripetuti debbano riutilizzare la promise memorizzata nella cache senza modificare il comportamento del batching; il lavoro è completo quando il comportamento è coperto da test mirati.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Expected Behavior

The docs state creating a new Promise and new key in the batchLoadFn for every load of the same key. When loading large datasets that can contain large amounts of duplicates this means we could call load(1) 100k times and we should get 1 total promise.

Current Behavior

The source code reveals that whether or not we use the cache, we still end up creating a new promise. https://github.com/graphql/dataloader/blob/main/src/index.js#L92 .

Possible Solution

Why not just return the cachedPromise?

Context

It looks like the change originates from https://github.com/graphql/dataloader/commit/06c403bd72abef401dd7619c8ae992f03ed6b7d3 . It's been many years, but the reasoning behind that change feels misguided maybe? As the code stands right now, isn't the value of "caching promises" entirely eliminated because we end up creating a new promise for every load call. So if 50k records all relate to the same record that means 50k promises, rather than 1. The trade-off for that loss is that we can re-use cache from an earlier call, but is that really used enough to warrant that behavior? The current code only provides value in the event we have a partially primed cache, where some objects are cached and others are not, ensuring that subsequent loads end up on the same tick. That feels like a pretty edgy-ish behavior that almost should be solved using different batch loading techniques.

Lingua principale
JavaScript
Stelle
13.4k
Fork
522
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di graphql/dataloader

Tutte le issue di graphql/dataloader

Issue simili

Altre issue su JavaScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.