Draft for avoiding carpet-banning utility types using module-based definitions (#208) #303
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 changeddeclare namespace
toexport namespace
.(I also changed the
Request<T>
return types to aPromise
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'sRecord
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" likegapi.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: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: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.