Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object/array as key #8

Closed
jardakotesovec opened this issue Oct 21, 2015 · 9 comments
Closed

Object/array as key #8

jardakotesovec opened this issue Oct 21, 2015 · 9 comments

Comments

@jardakotesovec
Copy link

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
Copy link

+1

Yes, something like this:

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

loader.load({phone, id})

@zerkalica
Copy link

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
Copy link
Contributor

leebyron commented Nov 3, 2015

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
Copy link

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

@jardakotesovec
Copy link
Author

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
Copy link
Contributor

leebyron commented Nov 3, 2015

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:
#8 (comment)

@zerkalica
Copy link

Any progress?

@leebyron
Copy link
Contributor

I will dig into this soon, thanks :)

@CatTail
Copy link

CatTail commented Dec 17, 2015

Any progress?

leebyron added a commit that referenced this issue Jan 11, 2016
As requested in #8 and implemented by #10, this allows an option for a custom cache key function which can be useful when objects or arrays are used as load keys, since JavaScript does not use value equality on those types.
leebyron added a commit that referenced this issue Jan 12, 2016
As requested in #8 and implemented by #10, this allows an option for a custom cache key function which can be useful when objects or arrays are used as load keys, since JavaScript does not use value equality on those types.
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

No branches or pull requests

4 participants