graphql / dataloader

DataLoader is a generic utility to be used as part of your application's data fetching layer to provide a consistent API over various backends and reduce requests to those backends via batching and caching.
MIT License
12.9k stars 515 forks source link

Object/array as key #8

Closed jardakotesovec closed 8 years ago

jardakotesovec commented 9 years ago

Would you consider supporting Object/array as key? Currently that will work apart from caching, because Map use === identity.

There are situations when I need to pass more than just simple number or string. In that case I have to stringify it and parse at the moment to be able use cache. Is that something that could be part of dataloader?

zerkalica commented 9 years ago

+1

Yes, something like this:

const loader = new DataLoader(({phone, id}) => myLoad({phone, id}))

loader.load({phone, id})
zerkalica commented 9 years ago

This fix is monkey-patch of global Map. Need to delegate serializeKey in options of dataloader.

import BaseMap from 'core-js/library/es6/map'

function objectToKey(obj) {
    return Object.keys(obj).sort().map(k => k + ':' + obj[k]).join('-')
}

function serializeKey(key) {
    let result
    if (typeof key === 'object' && key !== null) {
        result = Array.isArray(key)
            ? key.map(k => serializeKey(k)).join('-')
            : objectToKey(key)
    } else {
        result = key
    }
    return result
}

class Map extends BaseMap {
    get(key) {
        return super.get(serializeKey(key))
    }

    has(key) {
        const is = super.has(serializeKey(key))
        return is
    }

    delete(key) {
        return super.delete(serializeKey(key))
    }

    set(key, data) {
        return super.set(serializeKey(key), data)
    }
}

global.Map = Map
leebyron commented 9 years ago

Hmm, there are also cases where Objects (including Arrays) are used as identifying keys by reference. I certainly wouldn't want to also break that use-case.

Do you have a proposal for how we should differentiate between these cases? Also, this library's goal is to be singular in focus and with little to no dependencies. What might work for your needs?

zerkalica commented 9 years ago

Just extract cache key generating strategy from dataloader and allow to programmers define they own strategies.

jardakotesovec commented 9 years ago

Sounds good to me. Just having new serializeKey option in DataLoader options, which would be used if available. So by default it still would be comparing by reference and if needed we could pass custom serializeKey function.

leebyron commented 9 years ago

I like that idea. If anyone is interested in championing this, I would be happy to entertain the PR

— Sent from Mailbox

On Tue, Nov 3, 2015 at 2:45 PM, Jarda Kotěšovec notifications@github.com wrote:

Sounds good to me. Just having new serializeKey option in DataLoader options, which would be used if available. So by default it still would be comparing by reference and if needed we could pass custom serializeKey function.

Reply to this email directly or view it on GitHub: https://github.com/facebook/dataloader/issues/8#issuecomment-153356466

zerkalica commented 8 years ago

Any progress?

leebyron commented 8 years ago

I will dig into this soon, thanks :)

CatTail commented 8 years ago

Any progress?