Skip to content

chore: use TSTyche assertions in the document type test#16068

Merged
vkarpov15 merged 3 commits intoAutomattic:masterfrom
mrazauskas:migrate-document-type-test
Mar 12, 2026
Merged

chore: use TSTyche assertions in the document type test#16068
vkarpov15 merged 3 commits intoAutomattic:masterfrom
mrazauskas:migrate-document-type-test

Conversation

@mrazauskas
Copy link
Copy Markdown
Contributor

Summary

Same as #16067, but with some mismatch of expected types. Therefore, I think this needs an extra attention (see inline comments).

@mrazauskas mrazauskas changed the title chore: use TSTyche assertions in document type test chore: use TSTyche assertions in the document type test Mar 4, 2026
Comment thread test/types/document.test.ts Outdated

ItemSchema.pre('validate', function preValidate() {
ExpectType<Model<unknown>>(this.$model('Item1'));
expect(this.$model('Item1')).type.toBe<unknown>();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Model<unknown> does not pass. The type seems to be plain unknown. Is that correct?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably some type widening quirk - $model uses generics. I think we could just expect assignable here, or test const model: Model<unknown> = this.$model('Item1'), what do you think? I don't like expecting that the type is strictly unknown here

Comment thread test/types/document.test.ts Outdated
Comment on lines +306 to +307
expect(user.$model()).type.toBe<unknown>();
expect(user.model()).type.toBe<unknown>();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

typeof User or simply .type.toBe(User) does not pass. The type seems to be plain unknown. Is that correct?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similar type widening. Maybe we just check assignability?

@mrazauskas
Copy link
Copy Markdown
Contributor Author

mrazauskas commented Mar 10, 2026

@vkarpov15 Thanks for feedback.

I took a better look and found out the following. The argument of expect() is typed as unknown and both Document#$model() and Document#model() methods do not type argument constraints. Looks like TypeScript decides to pass unknown as type argument (since that is allowed). The resulting type becomes unknown.

Two possible solutions:

  1. Move the function call outside of the expect:

    const x = this.$model('Item1');
    expect(x).type.toBe<Model<unknown>>(); // pass
  2. Or add constraints to both signatures and the resulting type will be inferred correctly. See fix: add type constraints for Document#model() #16081

What sounds better for you?


Another detail. Right now ReturnType<typeof this.$model> is inferred as unknown. After adding the constraints (as suggested in #16081) it becomes Model<DocType> (the return type of last overload).

I try to show that other unrelated APIs infer the unknown as well. TSTyche does not use ReturnType anyhow.

@vkarpov15
Copy link
Copy Markdown
Collaborator

I merged #16081

@mrazauskas
Copy link
Copy Markdown
Contributor Author

mrazauskas commented Mar 10, 2026

I merged #16081

Thanks! Seems like all is working as expect. The unknowns are gone. See updated tests in d5cfb0a

@vkarpov15 vkarpov15 added this to the 9.3.1 milestone Mar 12, 2026
@vkarpov15 vkarpov15 merged commit 37ff792 into Automattic:master Mar 12, 2026
2 checks passed
@mrazauskas mrazauskas deleted the migrate-document-type-test branch March 12, 2026 16:47
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