Errors related to Request and Response types

[lumaxis]

I think #249 might have introduced an issue in the generated types of this package. When using the package in a project that uses TypeScript (versione 4.3.5), I get the following errors after updating to the latest version of @octokit/oauth-app:

node_modules/@octokit/oauth-app/dist-types/middleware/cloudflare/index.d.ts:4:36 - error TS2304: Cannot find name 'Request'.

4     onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
                                     ~~~~~~~

node_modules/@octokit/oauth-app/dist-types/middleware/cloudflare/index.d.ts:4:48 - error TS2304: Cannot find name 'Response'.

4     onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
                                                 ~~~~~~~~

node_modules/@octokit/oauth-app/dist-types/middleware/cloudflare/index.d.ts:4:67 - error TS2304: Cannot find name 'Response'.

4     onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
                                                                    ~~~~~~~~

node_modules/@octokit/oauth-app/dist-types/middleware/cloudflare/index.d.ts:5:15 - error TS2304: Cannot find name 'Request'.

5 }): (request: Request) => Promise<Response>;
                ~~~~~~~

node_modules/@octokit/oauth-app/dist-types/middleware/cloudflare/index.d.ts:5:35 - error TS2304: Cannot find name 'Response'.

5 }): (request: Request) => Promise<Response>;

I guess the Request and Response types need to be explicitly imported?

[wolfy1339]

Thanks for bringing this up!

It's odd that our tests didn't raise this issue earlier.

Would you like to send in a PR to fix the issue?

[lumaxis]

@wolfy1339 Sure, happy to! I'm just not quite sure if what I suggested is the proper correct fix – or rather: Why does this break in the first place? It seems to only fail in this one file but not in others that use a similar pattern 🤔

[wolfy1339]

It seems to use those types from the @types/node-fetch package

Adding an explicit import should fix the problem

[baoshan]

It seems to only fail in this one file but not in others that use a similar pattern.

Only middleware/cloudflare/* relies on Request/Response types.

I’m not sure importing types from @types/node-fetch is our best option. TypeScript includes a default set of type definitions for built-in JS APIs. Request type is included in both "DOM" and "WebWorker", the prior one is included by default (when there is no explicit compilerOptions.lib field in tsconfig.json).

Without the DOM lib, handle-request.ts should also raise errors since it relies on URL.

I’m not sure what does your compilerOptions.lib look like. Please let me know if this repo does needs a fix.

[gr2m]

yeah these are tricky if we want universal code. I set fetch to any because I ran into problems such as this: https://github.com/octokit/types.ts/blob/master/src/Fetch.ts

[wolfy1339]

Since we switched to using native fetch, is there anything we can do for this issue?

Typescript now has the types for Fetch included for NodeJS in the global scope. Request and Response are defined.