Add context option

[szmarczak]

Fixes #740

Checklist

• I have read the documentation.
• I have included a pull request description of my changes.
• I have included some tests.
• If it's a new feature, I have included documentation updates.

[sindresorhus]

We also should discuss making it unknown vs object. The benefit of unknown is that it can be any type. However, the benefit of having it always be an object is that we potentially could merge the object for the user.

[sindresorhus]

// @juhovh

[sindresorhus]

We also should discuss making it unknown vs object. The benefit of unknown is that it can be any type. However, the benefit of having it always be an object is that we potentially could merge the object for the user.

@szmarczak @juhovh @sholladay Any thoughts on this? I guess most will want an object here as they would want to name the data, for example .userData.unicorn instead of just .userData, which is not that readable. The merging factor is also tempting, but how far do we go, shallow merge, deep merge, or no merge at all? Any potential downsides of shallow merging?

[sindresorhus]

@szmarczak => #777 (comment)

[szmarczak]

@szmarczak => #777 (comment)

Ah, forgot about that one.

I guess most will want an object here as they would want to name the data, for example .userData.unicorn instead of just .userData, which is not that readable.

👍

but how far do we go, shallow merge, deep merge, or no merge at all? Any potential downsides of shallow merging?

Shallow merge looks good to me. If someone wants deep merge we can discuss this later.

[sholladay]

I would rather name this app instead of userData. Not only is it shorter, it feels more familiar to me, because hapi has this same exact feature named server.app.

Provides a safe place to store server-specific run-time application data without potential conflicts with the framework internals.

As for its type, I would probably go for unknown and avoid merging. If for no other reason than just the simplicity. Also, since merging usually requires some opinionated tradeoffs, it is worth keeping in mind that we don't "own" this namespace, so we should avoid being overly opinionated with it. In the end, though, I see this as something to be iterated on after more real-world experience.

[sindresorhus]

I would rather name this app instead of userData. Not only is it shorter, it feels more familiar to me, because hapi has this same exact feature named server.app .

There's a big difference though. hapi is a server framework, not a request client. Got is used in many situations where there's no concept of "app". I'm pretty content with the name userData. It makes it clear it's for the user and that it's for storing data.

As for its type, I would probably go for unknown and avoid merging. If for no other reason than just the simplicity.

How about object and no merging? If we go with unknown now and later realize object is better, we'll need to do a breaking change, which will inconvenience users. I also think the readability point is an important factor: console.log(response.userData) vs console.log(response.userData.unicorn).

[juhovh]

Sorry for not commenting earlier, but I wasn’t quite sure then what would be the best option here. After giving it some thought, I believe this userData is shared between all different hooks, right? Unknown would make sense if it would be per hook, but shared data shouldn’t be of unknown type IMHO. Having an object with unknown values is fine, one argument for doing a shallow merge would be that one could throw an error in case of a conflict between hooks. For the same reason deep merge sounds wrong, since got doesn’t own the data that the hook sets. It all boils down to which part of the code owns what in the end.

[sholladay]

FWIW, when I first saw this feature named userData, I honestly assumed it had something to do with the user portion of a URL (e.g. username and password). My second thought was that maybe it has something to do with extending got and customizing its default options. Perhaps that's just me, though? At any rate, now that I understand it, it very much feels like application state, not something that's necessarily static data. And above, the term app is used in a pretty loose sense.

How about object and no merging?

Yeah, I was thinking of suggesting that. Sounds good to me. 👍

[szmarczak]

Regarding shallow merge:

const a = got.extend({
    userData: {
        unicorn: {
            value: "this won't get merged"
        },
        catSays: "meow"
    }
});

const b = a.extend({
    userData: {
        unicorn: {
            color: "rainbow"
        },
        dogSays: "woof"
    }
});

// Result:
{
    unicorn: {
        color: "rainbow",
    },
    catSays: "meow",
    dogSays: "woof"
}

There's one con. Should arrays be merged? Or replaced?

So yeah, it can be a object with no merge mechanics. We could simply emit old user data and new user data so the user can decide...

[szmarczak]

Maybe let's leave it unknown? If people want to they can use it as an object too.

[sindresorhus]

FWIW, when I first saw this feature named userData, I honestly assumed it had something to do with the user portion of a URL (e.g. username and password). My second thought was that maybe it has something to do with extending got and customizing its default options. Perhaps that's just me, though?

Any other naming suggestions? I don't really want the app word in it, but we can maybe think of something else.

• contextData
• customData
• contextObject
• context
• environment
• environmentObject (This is what SwiftUI uses for shared data: https://developer.apple.com/documentation/swiftui/environmentobject)

Yeah, I was thinking of suggesting that. Sounds good to me. 👍

Cool. Let's make it an object and no merging. TS typed as {[key: string: unknown}.

Actually, should we make it generic? context<T extends {[key: string: unknown}>. That way users could make it more strictly typed if they wanted. Or am I overthinking it?

Another question, should it be a Map? That's usually a better data structure for key/value.

[szmarczak]

IMO customData is too general. The type of the data should be specified. context / contextData / contextObject sounds good to me. When I hear environment I immediately think of environment vars.

Actually, should we make it generic?

👍

should it be a Map?

It depends on the usage. Or maybe let's accept both: generic objects and maps?

[sindresorhus]

It depends on the usage. Or maybe let's accept both: generic objects and maps?

I think we should pick one. If we make it a union, users will have to type assert it, which is annoying.

[sindresorhus]

I'm leaning towards contextData, but I will let the other people chime in first.

[sholladay]

context and contextData are my favorite, in that order. My preference is only mild.

[szmarczak]

Both context and contextData sound good to me. Is it possible to do pools in GitHub?

[sindresorhus]

@sholladay Thoughts on Map vs object? I’m leaning towards Map.

[sindresorhus]

Let’s go with context.

[sindresorhus]

Hmm, Map would make it much harder to manually extend if the user wanted to do so. Let's go with object.

[szmarczak]

How do I make this generic?