Ky: Discussion: Is it worth it to keep self. or window. for DOM APIs ?

[GrosSacASac]

https://github.com/sindresorhus/ky/blob/master/index.js#L113

We could completely drop self. in self.Headers

• More concise code
• Makes it potentially easier to transform into universal library
• What if NodeJs (or other runtime) have native support for fetch, Headers in the future
• Save a few bytes
• not having to think about self. or window. https://github.com/sindresorhus/ky/pull/36

Why not drop it ?

• Pattern to distinguish between DOM APIs and ES API

[sindresorhus]

More concise code

I don't really mind self. It makes it more clear that those are globals.

Makes it potentially easier to transform into universal library

What does this mean?

What if NodeJs (or other runtime) have native support for fetch, Headers in the future

Not going to optimize for something that may or may not happen in the future.

Save a few bytes

After minification and gzip that will be close to zero.

[GrosSacASac]

by universal I mean Code that runs the same in the Browser and Server. But that is not the most relevant argument for this repository. But if you can make code more universal for free ...

I don't really mind self. It makes it more clear that those are globals.

Should we write self.Array.isArray, self.TypeError, self.String ? These are also globals.

[ArsalanDotMe]

I agree about dropping self because this makes Ky not run in react-native environment. React native was supposed to have good support for the fetch API and Ky was based on fetch so I assumed it would work there too, but because of self it just throws errors.

[sindresorhus]

They need to be properties of an object so they work with node-fetch in testing and if someone wants to use Ky with node-fetch on the Node.js side.

We could use a small helper to get the global though, something like:

const thisGlobal = () => {
    if (typeof self !== 'undefined') {
        return self;
    } else if (typeof window !== 'undefined') {
        return window;
    } else if (typeof global !== 'undefined') {
        return global;
    }
};

I assume that would work in React Native?

[deadNightTiger]

Once you ruled out the browser, just use Function("return this")() IMO. That does not work only in browsers when CSP is blocking the Function constructor.

[sindresorhus]

@deadNightTiger That's less code, yes, but not a good solution. CSP support is also important.

[deadNightTiger]

@sindresorhus Is CSP a thing outside of browsers? I'm only suggesting to use Function only after testing for self. Alternatively, we could do a try-catch and use self if Function has failed.

[sindresorhus]

@deadNightTiger What is the benefit over what I've posted in https://github.com/sindresorhus/ky/issues/53#issuecomment-429611316? It's not worth using Function("return this")() only to save a few LOC.

[deadNightTiger]

I guess there's not much of a benefit practically, yeah. In theory Function("return this")() is the only portable method (until https://github.com/tc39/proposal-global ships), not relying on specific environments detection. But this library is already targeted at a specific environment.

[sscotth]

Tried 0.5.1 with react native

Can't find variable: document
- node_modules/ky/index.js:138:82 in Ky
- node_modules/ky/index.js:269:42 in <unknown>