Draft for avoiding carpet-banning utility types using module-based definitions (#208)

[HoldYourWaffle]

This is my initial concept for how we could avoid carpet-banning utility types (#208) using module-based type definitions. This PR is purely meant as a "proof-of-concept"-ish thing, and definitely not intended for merge (which would make no sense).

I basically took the original defintions, wrapped it inside a declare module 'gapi' block (module augmentation), and changed declare namespace to export namespace. (I also changed the Request<T> return types to a Promise as a workaround for the fact that I couldn't/was too lazy to figure out what they're supposed to refer to.)

As you can see, I define (but don't export) a $TSRecord type alias, which can safely refer to TypeScript's Record and be safely referred to from Google's types. Because this alias is defined inside a module declaration without being exported, it doesn't pollute the global scope.

The breaking changes with this draft are very limited, at least as far as I can tell (which isn't very far at all). The types are no longer defined globally, so users will have to add an import { gapi } from 'gapi' statement. This will automatically import types from "plugins" like gapi.client.chromeuxreport. I think this is an improvement compared to the current situation, where I had to add individual imports for each "plugin" like this:

import 'gapi'
import 'gapi.client.chromeuxreport'

A downside of this way of doing things is the fact that @types/gapi will have to be "modularised" as well, which isn't under "our control" as far as I know. To get the module augmentation & declaration merging working, its definitions will also have to be structured as a module, like this:

declare module 'gapi' {
    export interface GoogleApiOAuth2TokenObject { ... }
    export interface GoogleApiOAuth2TokenSessionState { ... }

    export namespace gapi { ... }
    export namespace gapi.auth { ... }
    export namespace gapi.client { ... }
}

Just like our packages, this removes the types from the global scope and (might) introduce the need for an import statement. This is a breaking change, but as far as I know this is a best-practice and shouldn't (although you never know with DT) lead to much friction.

Note: I'm not 100% certain if the declare module wrappers are necessary, but they were necessary to get things working in my test-setup. I think they were necessary in my setup because I perhaps wasn't packaging the definitions 100% correctly, and that shouldn't be an issue on DT of course.

[Maxim-Mazurok]

I think this is an improvement compared to the current situation, where I had to add individual imports for each "plugin" like this ...

Since they are currently global, I don't think that manually importing should be necessary. Also, importing types only may introduce some issues with linters/plugins/packers, but this should be investigated, I'm not sure.

(I also changed the Request<T> return types to a Promise as a workaround for the fact that I couldn't/was too lazy to figure out what they're supposed to refer to.

This is from @types\gapi.client, FYI.

A downside of this way of doing things is the fact that @types/gapi will have to be "modularised" as well, which isn't under "our control" as far as I know

That's right, @types/gapi and @types/gapi.auth2 are not under "our control" and both are pretty important packages. We have an outdated substitute for both of them: @types/gapi.client. Currently, it doesn't provide much value, AFAIK, but my WIP Un-minified GAPI project should change this. BTW, feel free to join me in reverse-engineering gapi, it's quiet fun, god practice for finding patterns in code :) api.js is almost done, but the most interesting stuff happens in client.

---

It's an interesting approach and seems about right. But since it's a pretty major breaking change, I think that it'll be better to evaluate this after we have @types/gapi.client updated.

Another thought: we can do this without breaking anything, but by polluting the global namespace. For example, see how yargs did this. You can use yargs.InferredOptionType, while perhaps you shouldn't be able to.

[HoldYourWaffle]

Since they are currently global, I don't think that manually importing should be necessary.

As I said in #208 as well, this might've been caused by my configuration (Vue SFC's), I'll have to investigate that.

This is from @types\gapi.client, FYI.

Noted 😉

BTW, feel free to join me in reverse-engineering gapi, it's quiet fun, god practice for finding patterns in code :)

I would love to help, but I unfortunately lack the time to do so (as well as a good chunk of experience on how things are "generally done in JS", which I imagine is a must when reverse-engineering).

It's an interesting approach and seems about right. But since it's a pretty major breaking change, I think that it'll be better to evaluate this after we have @types/gapi.client updated.

I 100% agree. This definitely isn't a pressing issue either, since types can just be inlined in the meantime.

Another thought: we can do this without breaking anything, but by polluting the global namespace. For example, see how yargs did this. You can use yargs.InferredOptionType, while perhaps you shouldn't be able to.

Ew...

You're right of course, it is a possibility, but I'd only go with that option as a last resort. We'd have to create a "$TSxxx"-ish definition for each utility type that's used in any module, and put that in some "main package" like @types/gapi.client to avoid duplicatation-conflicts. As well as the fact that, well, we'd pollute the global namespace, which I think is (almost) never a good thing really.