Skip to content

fix: enrich type definition of buildCompilerFromPool method#156

Merged
Eomm merged 2 commits intofastify:mainfrom
gianmarco27:chore/ajv-factory-types
Oct 13, 2025
Merged

fix: enrich type definition of buildCompilerFromPool method#156
Eomm merged 2 commits intofastify:mainfrom
gianmarco27:chore/ajv-factory-types

Conversation

@gianmarco27
Copy link
Contributor

@gianmarco27 gianmarco27 commented Oct 11, 2025

Enriches type definition for buildCompilerFromPool method to better represent fastify/ajv-compiler features.

Checklist

@gianmarco27 gianmarco27 force-pushed the chore/ajv-factory-types branch from 791e134 to cc138b9 Compare October 11, 2025 14:49
@gianmarco27 gianmarco27 force-pushed the chore/ajv-factory-types branch from cc138b9 to 500b173 Compare October 11, 2025 14:49
@gianmarco27 gianmarco27 mentioned this pull request Oct 11, 2025
Closed
4 tasks
mcollina
mcollina approved these changes Oct 11, 2025
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Eomm
Eomm approved these changes Oct 11, 2025
View reviewed changes
Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

The TS error is unrelated with this PR

@fastify/typescript do you have a quick fix for this?
I saw it is related to this:
typescript-eslint/typescript-eslint#3325

I would like to fix it before publishing

@Eomm Eomm changed the title chore: enrich type definition of buildCompilerFromPool method fix: enrich type definition of buildCompilerFromPool method Oct 11, 2025
@Eomm Eomm added the bug Something isn't working label Oct 11, 2025
@gianmarco27
Copy link
Contributor Author

gianmarco27 commented Oct 12, 2025

I have been reading through a set of issues regarding this false positive
(e.g. typescript-eslint/typescript-eslint#10266) but yet i didn't find any option to tune it.

I think however using declare there might actually be the issue, telling the compiler that this function already exists in the code, while buildCompilerFromPool does not actually exist in the code outside of the return scope of the AjvCompiler function.

So my proposal here would be to convert the two signature definitions to just types.

Follows commit (We can drop it if you disagree on this solution)

@Eomm Eomm merged commit 991b32a into fastify:main Oct 13, 2025
14 checks passed
This was referenced Oct 15, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollina mcollina mcollina approved these changes

@Eomm Eomm Eomm approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gianmarco27 @mcollina @Eomm