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

/// <reference lib="dom" /> in api-call package is globally overriding types #507

Closed
AdrianGonz97 opened this issue May 8, 2023 · 3 comments
Labels
bug semver-major Issues that need a major version bump to be released (i.e. a breaking change)

Comments

@AdrianGonz97
Copy link
Contributor

Bug Report

I'm using @twurple/api and @twurple/auth in a Cloudflare Worker. CF Workers provide additional runtime APIs such as crypto.subtle.timingSafeEqual so I have to use their library typings to get proper type support for them.

The issue is that /// <reference lib="dom" /> is being used in @twurple/api-call which is globally overriding the CF Worker types.

I've tracked down the two instances where it's being used:

Code

Minimal Reproduction

Steps:

Clone the repo:

git clone https://github.com/AdrianGonz97/twurple-type-override-reproduction

Install dependencies:

npm i

You can comment in and out line 6: import { ApiClient } from "@twurple/api"; to see the override occur. If anything is imported from Twurple, crypto.subtle.timingSafeEqual will show a type error.

Actual Behavior

lib.dom.d.ts is being imported and is globally overriding types.

Environment

  • Version: 6.2.0
  • Node version: 18.15.0
  • Operating system: WSL Ubuntu
@d-fischer
Copy link
Member

As the types used are inherently global, do you have a better solution for this?

@AdrianGonz97
Copy link
Contributor Author

@d-fischer Sure! One way we can accomplish this would be to use and import the @types/web package as a replacement for the triple slash lib directive to acquire the DOM types.

I've already tested this out with Twurple and it works perfectly! It no longer overrides the types globally. You can checkout how my reproduction is no longer showing any compile errors here. Just clone and switch branches with git checkout fix/using-@types/web.

To add the package, we just run the following:

cd packages/api-call

# it's fine to add this as a devDependency
yarn add @typescript/lib-dom@npm:@types/web --dev 

We then remove the directive /// <reference lib="dom" /> from both packages/api-call/src/apiCall.ts and packages/api-call/src/TwitchApiCallOptions.ts.

Finally, we just need to import the following at the top of the file (we only need to import it once):

// packages/api-call/src/TwitchApiCallOptions.ts

import '@typescript/lib-dom';

@d-fischer
Copy link
Member

d-fischer commented May 21, 2023

While implementing this, I've found a few issues with passing fetch options, so I'm implementing a different solution now:

  • The internal types don't depend on the dom lib, the @types/web package or anything else anymore. Instead, the smallest common denominator was extracted into and exported from the @d-fischer/cross-fetch package.
  • If you need to pass fetch options that are specific to one platform, you can now augment the TwitchApiCallFetchOptions type (this is possible because it was changed to an interface)
  • This is a breaking change for all current users of the fetchOptions config option (but I'm pretty sure your approach would be too in some obscure way), so it will only be implemented in version 7.0.

Stay tuned for a new prerelease version within the next week.

@d-fischer d-fischer added the semver-major Issues that need a major version bump to be released (i.e. a breaking change) label May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug semver-major Issues that need a major version bump to be released (i.e. a breaking change)
Projects
None yet
Development

No branches or pull requests

2 participants