Skip to content

fix(types): fix 388 type errors and fully type src/lib/functions/#7130

Merged
serhalp merged 31 commits intomainfrom
fix/functions-types
Apr 7, 2025
Merged

fix(types): fix 388 type errors and fully type src/lib/functions/#7130
serhalp merged 31 commits intomainfrom
fix/functions-types

Conversation

@serhalp
Copy link
Copy Markdown
Member

@serhalp serhalp commented Mar 25, 2025

Summary

This started as a bump of https://github.com/netlify/local-functions-proxy/pull/137 to pull in the now-typed version of @netlify/functions-proxy but devolved into fixing hundreds of types.

This fully types src/lib/functions/ and fixes a number of core types used across the codebase.

These types revealed various minor runtime errors / bugs that I also fixed, but in a separate PR that I'll open stacked on this one. I tried to mostly keep behavioural changes for that follow-up PR, but I did include a few here, which I'll call out in inline comments.

Reviewing

Sorry this is big and sorry I didn't break this down into smaller commits much. I'd recommend reviewing the smaller commits first and finishing with fix(types): fix functions types and much more then fix: fix functions regression and fully type lib/functions/.

To do

  • Fix the skipped test in tests/integration/commands/dev/dev-miscellaneous.test.ts
  • Fix the strange Host machine does not support local functions proxy server failure on ubuntu-latest node 18.17.0
  • Maybe release a few netlify/build fixes and bump here to fix a few more errors (or just do as a follow-up)

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 25, 2025

📊 Benchmark results

Comparing with 919c941

  • Dependency count: 1,173 (no change)
  • Package size: 284 MB ⬆️ 0.02% increase vs. 919c941
  • Number of ts-expect-error directives: 426 ⬇️ 66.67% decrease vs. 919c941

Comment thread src/commands/init/init.ts
@serhalp serhalp force-pushed the fix/functions-types branch 6 times, most recently from cd97ce2 to f0f36a6 Compare March 28, 2025 21:20
@serhalp serhalp changed the title fix(types): fix 200+ type errors and fully type src/lib/functions/ fix(types): fix 295 type errors and fully type src/lib/functions/ Mar 28, 2025
@serhalp serhalp force-pushed the fix/functions-types branch 13 times, most recently from 489d51d to d350f09 Compare April 2, 2025 23:19
@serhalp serhalp changed the title fix(types): fix 295 type errors and fully type src/lib/functions/ fix(types): fix 388 type errors and fully type src/lib/functions/ Apr 2, 2025
@serhalp serhalp force-pushed the fix/functions-types branch 3 times, most recently from 7b2f737 to fd95e71 Compare April 3, 2025 12:58
Comment thread bin/run.js
Comment thread src/commands/api/api.ts
Comment on lines -27 to -29
if (!siteData) {
error(`Unable to process site`)
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was just compensating for incorrectly refined types

Comment on lines -40 to +43
Name: user?.full_name,
Email: user?.email,
Name: user.full_name,
Email: user.email,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was compensating for incorrectly refined types

@serhalp serhalp requested a review from ndhoule April 7, 2025 19:21
Comment thread package.json Outdated
}

public get(key: string): T[typeof key] {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is due to an indirect merge conflict from the rebase on #7165. I had removed a bunch of file-wide eslint suppressions for this file, but that PR reintroduced some. I didn't want to bother digging too much here (T[type of key] is any by design...).

Copy link
Copy Markdown
Contributor

@ndhoule ndhoule left a comment

Choose a reason for hiding this comment

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

🚢

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