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

fix(events,fetch): avoid polluting global types #427

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

Rugvip
Copy link
Contributor

@Rugvip Rugvip commented Mar 16, 2023

Description

Sorry for going straight for a PR, I figured this fix was best described with code.

We're seeing issues with trying to use libraries that both declare global symbols such as _fetch, in this case this library along with cross-fetch. Due to these libraries both declaring this same global type, TypeScript will treat this as an error simply if both dependencies are installed in the same project. Using --skipLibCheck is not an option as this is an important check that we want to keep.

There are other solutions to this issue, but I figured the one I propose is the cleanest. globalThis has been available in TypeScript since 3.4, released 4 years ago.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

  • yarn run ts:check
  • The following manual procedure:
    • Install both @whatwg-node/fetch and cross-fetch in the same project

    • Run tsc, with --skipLibCheck=false. Multiple type errors are produced, for example:

      node_modules/@whatwg-node/fetch/dist/index.d.ts:8:15 - error TS2451: Cannot redeclare block-scoped variable '_fetch'.
      
      8 declare const _fetch: typeof fetch;
                      ~~~~~~
      
        node_modules/cross-fetch/index.d.ts:3:15
          3 declare const _fetch: typeof fetch;
                          ~~~~~~
          '_fetch' was also declared here.
      
    • Replace the contents of node_modules/@whatwg-node/fetch/dist/index.d.ts in the project with the contents from this PR.

    • Run tsc again, the errors are now gone and no new errors appear.

I applied the same fix to events to keep things consistent, but have not tested it in the same way.

Test Environment:

  • OS: MacOS 13.2.1
  • typescript: 4.7.4
  • NodeJS: v16.14.2

Checklist:

  • I have followed the
    CONTRIBUTING doc and the
    style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests and linter rules pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants