Skip to content

feat: expose generatedFunctions to consumers #6525

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eduardoboucas
Copy link
Member

@eduardoboucas eduardoboucas commented Jul 4, 2025

Summary

Follow-up to #6487, exposing the list of generated functions to consumers of @netlify/build via the default export and the startDev method.

This will be used by the CLI to learn about any functions that were generated and that need to be added to the list of function paths.

I ended up changing the internals a little bit such that we keep passing returnValues around internally, but what we expose externally is a generatedFunctions object. The generation of this object has been moved to a separate file, which is now reused by the functions core step and the logic that prints functions. This is for unification and simplicity purposes, no functional changes are expected.

A picture of a cute animal (not mandatory, but encouraged)

🦅

Copy link
Contributor

github-actions bot commented Jul 4, 2025

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

Copy link
Contributor

github-actions bot commented Jul 4, 2025

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

@eduardoboucas eduardoboucas marked this pull request as ready for review July 4, 2025 16:14
@eduardoboucas eduardoboucas requested a review from a team as a code owner July 4, 2025 16:14
Copy link
Member

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +256 to +260
const result: Record<string, GeneratedFunction[]> = {}

for (const func of generatedFunctions) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
result[func.generator.name] = result[func.generator.name] || []
Copy link
Member

@serhalp serhalp Jul 4, 2025

Choose a reason for hiding this comment

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

FWIW this is because given f: Record<string, Foo> with noUncheckedIndexAccess disabled, f[string] results in the type Foo, not Foo | undefined. ESLint is being pedantically correct here (arguably).

You can avoid the suppression this way:

Suggested change
const result: Record<string, GeneratedFunction[]> = {}
for (const func of generatedFunctions) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
result[func.generator.name] = result[func.generator.name] || []
const result: Record<string, GeneratedFunction[] | undefined> = {}
for (const func of generatedFunctions) {
result[func.generator.name] = result[func.generator.name] || []

Copy link
Member Author

Choose a reason for hiding this comment

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

But that's not the type I want to return. I don't want consumers of this method to think that some keys will have the value undefined, because that shouldn't be the case.

Copy link
Member

Choose a reason for hiding this comment

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

It's problematic either way. It's just a limitation of TypeScript's type system without noUncheckedIndexAccess:

  • With your type, a user dereferencing result['kjfandsakj'] will get GeneratedFunction[].
  • With my type, a user iterating over result's own keys and dereferencing result[key] will get GeneratedFunction[] | undefined.

Both cases are not what we intend to express 😞. The latter is safer, but way more annoying.

https://claude.ai/share/a8fbb3cb-bd72-4761-a2e6-1d55e876889c

The only good solution is enabling noUncheckedIndexAccess.

(To be clear, this is absolutely nonblocking tangential rambling—please proceed with merging!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants