discordjs / discord.js-modules

Modularisation of discord.js (WIP)
Apache License 2.0
193 stars 30 forks source link

fix(Rest): remove DOM reference types #97

Closed cherryblossom000 closed 2 years ago

cherryblossom000 commented 2 years ago

Please describe the changes this PR makes and why it should be merged:

This reverts #55 and instead augments the global namespace with the missing URL and URLSearchParams (see https://github.com/DefinitelyTyped/DefinitelyTyped/issues/34960). This means that projects that use @discordjs/rest won't need to have the global namespace polluted with DOM typings (and also things like document.querySelector('.oh-no') will compile even in a Node.js environment!), even if "dom" isn't included in the user's own TSConfig.

Status and versioning classification:

codecov[bot] commented 2 years ago

Codecov Report

Merging #97 (7776c67) into main (9070440) will increase coverage by 0.07%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   88.82%   88.89%   +0.07%     
==========================================
  Files           9       10       +1     
  Lines        1584     1594      +10     
  Branches      167      167              
==========================================
+ Hits         1407     1417      +10     
  Misses         13       13              
  Partials      164      164              
Impacted Files Coverage Δ
packages/rest/src/lib/RequestManager.ts 93.11% <100.00%> (+0.01%) :arrow_up:
packages/rest/src/lib/global.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9070440...7776c67. Read the comment docs.

kevinlul commented 2 years ago

I second this change. I unexpectedly discovered DOM types polluting the global namespace, meaning that TypeScript will happily compile stuff like fetch and atob, only to fail at runtime with a ReferenceError. This shouldn't happen as this is a Node package.