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

Emit types #442

Merged
merged 5 commits into from
Mar 4, 2021
Merged

Emit types #442

merged 5 commits into from
Mar 4, 2021

Conversation

Rich-Harris
Copy link
Member

So far just app-utils. This fails in weird ways. If running for the first time, this happens:

➜  packages/app-utils git:(emit-types) npx tsc
../../node_modules/.pnpm/@types/node-fetch@2.5.8/node_modules/@types/node-fetch/index.d.ts:18:27 - error TS2792: Cannot find module 'form-data'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?

18 import FormData = require('form-data');
                             ~~~~~~~~~~~


Found 1 error.

I have no idea what's going on — form-data and node-fetch aren't referenced anywhere inside app-utils. If I run tsc --listFiles and tsc --traceResolution it goes berserk. I have no idea what it thinks it's trying to do.

Run it a second time, and this happens:

➜ packages/app-utils git:(emit-types) npx tsc
error TS5055: Cannot write file '/path/to/kit/packages/app-utils/files/index.d.ts' because it would overwrite input file.

error TS5055: Cannot write file '/path/to/kit/packages/app-utils/http/get_body/index.d.ts' because it would overwrite input file.

error TS5055: Cannot write file '/path/to/kit/packages/app-utils/http/get_body/read_only_form_data.d.ts' because it would overwrite input file.

error TS5055: Cannot write file '/path/to/kit/packages/app-utils/http/index.d.ts' because it would overwrite input file.


Found 4 errors.

I can't figure out how to tell it 'these are files YOU created, you utter dolt, it's okay to overwrite them'. Any tips?

@GrygrFlzr
Copy link
Member

I have no idea what's going on — form-data and node-fetch aren't referenced anywhere inside app-utils.

I hunted down the FormData thing and apparently it's this line in @types/node-fetch which imports form-data:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/a2bf82bb4fa9bd19495102986604ef4c7d8a0cd2/types/node-fetch/index.d.ts#L18

However, the only thing in the monorepo that depends on @types/node-fetch is packages/kit. I'm not sure if it's freaking out with the monorepo layout and trying to build the entire monorepo.

I can't figure out how to tell it 'these are files YOU created, you utter dolt, it's okay to overwrite them'. Any tips?

I fixed it by following its suggestion to use "moduleResolution": "node" and to explicitly specify that the input files must have a .js extension.

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these look like good fixes to me. thanks!

@Rich-Harris
Copy link
Member Author

Man, I could have sworn I tried both those things! Though my globs looked more like "files/**/*.js" than these. Anyway, thanks — will try and get kit emitting types before merging this

@@ -1,7 +1,7 @@
/** @type {import('./router').Router} */
/** @type {any} */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this from Router?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the comment still exists once these files have been bundled, but no longer points at anything. If TypeScript were to check that file, because of whatever configuration, it would get confused. Changing it to any doesn't actually impede us in any way, and doesn't affect users because Router and Renderer are a hidden implementation detail

@Rich-Harris Rich-Harris changed the title [WIP] Emit types Emit types Mar 4, 2021
@Rich-Harris Rich-Harris merged commit b800049 into master Mar 4, 2021
@Rich-Harris Rich-Harris deleted the emit-types branch March 4, 2021 22:41
@GrygrFlzr GrygrFlzr mentioned this pull request Mar 5, 2021
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.

3 participants