Skip to content

Conversation

@pierrecabriere
Copy link

What?

Added proper type detection to the global data fetching utilities in the form builder example. Enhanced the getGlobal and getCachedGlobal functions to leverage TypeScript generics for improved type safety.

Why?

This change improves developer experience by providing proper TypeScript type inference when working with global data. With these changes:
The returned data from getGlobal is now properly typed as DataFromGlobalSlug based on the slug parameter
TypeScript can now correctly infer the return type of getCachedGlobal based on the global slug
This prevents type errors and provides better IDE autocomplete/IntelliSense when accessing properties on retrieved global data

How?

Added import for the DataFromGlobalSlug type from Payload
Made getGlobal a generic function that accepts a type parameter T extends Global
Added proper return type annotation (Promise<DataFromGlobalSlug>) to the getGlobal function
Updated getCachedGlobal to use the same generic typing pattern
These changes maintain the same runtime behavior while enhancing type safety

Copy link
Contributor

@paulpopus paulpopus left a comment

Choose a reason for hiding this comment

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

Thank you for working through this, definitely an improved change.

In the instances where these functions are then used, could you please remove the type assignment? Eg for the header

Changing

const headerData: Header = await getCachedGlobal('header', 1)()

To

const headerData = await getCachedGlobal('header', 1)()

And removing the imports please?

@paulpopus paulpopus self-assigned this Mar 11, 2025
Copy link
Contributor

@paulpopus paulpopus left a comment

Choose a reason for hiding this comment

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

Thank you so much!

@paulpopus paulpopus enabled auto-merge (squash) March 12, 2025 23:02
@paulpopus
Copy link
Contributor

There's a werid CI error, can you pull the latest main and merge it into this branch please? @pierrecabriere

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

Labels

None yet

2 participants