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

Convert more adapter files to TypeScript #138

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Conversation

benmccann
Copy link
Member

@benmccann benmccann commented Nov 16, 2020

Makes more progress on #136

index.d.ts
server.js
server.d.ts
Copy link
Member

Choose a reason for hiding this comment

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

Can you also update .prettierignore at the root of the monorepo accordingly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Neither of these files actually have any exports so I think we could just skip typings here (see #139)

Copy link
Member Author

Choose a reason for hiding this comment

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

I sent #151 so that we don't have to update .prettierignore, which I think is a good idea whether or not we do #139. Anyway, I'm going to skip updating .prettierignore here since we now have a couple of other solutions for that

input: 'src/index.ts',
output: {
file: 'index.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

it's confusing that this index.js is built from TS whereas the one in adapter-netlify isn't. could we convert the netlifly one to TS too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree that we should convert adapter-netlify. I'm waiting for a new release of tiny-glob before converting it though (#136 (comment))

@ehrencrona
Copy link
Contributor

ehrencrona commented Nov 17, 2020

Note that there's a couple of build errors due to mismatch with types from app-utils; these are fixed by #121

e.g. adapter-node/src/server.ts(41,6): semantic error TS2322: Type 'string | undefined' is not assignable to type 'Method'.

@benmccann benmccann force-pushed the more-typescript branch 2 times, most recently from 2c475f4 to 33f527e Compare November 19, 2020 16:57
@benmccann
Copy link
Member Author

I've rebased against master so there shouldn't any more build errors

@benmccann
Copy link
Member Author

I'm going to go ahead and merge this to unblock my other work, but feel free to leave more comments if you have additional feedback and I'll fix in a follow up

@benmccann benmccann merged commit 05bb61b into master Nov 20, 2020
@benmccann benmccann deleted the more-typescript branch November 20, 2020 00:19
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