fix(type-helpers): apply deep partial substitution to plugin metadata - #4030
Merged
kamilmysliwiec merged 1 commit intoAug 17, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
PR Type
What is the current behavior?
Issue Number: N/A
DeepPartialTypeonly walks properties that carry explicit@ApiProperty()metadata. For a DTO written with the CLI plugin, the properties live in the generated metadata factory instead, so two things go wrong:ApiProperty, so nested DTOs keep their original fully required type.isDtoClassdecides whether a property type is a DTO by readingAPI_MODEL_PROPERTIES_ARRAYoff the prototype. A plugin generated DTO has nothing there until the schema is explored, so nested plugin DTOs are not recognized as DTOs at all and the recursive wrapping is skipped.The net effect is that
DeepPartialTypebehaves likePartialTypefor anyone using the plugin, which is the recommended setup. Nested fields stay required.applyMetadataFactorydoes not compensate for this. It applies the factory asApiProperty, but it does not replacetype: () => InnerwithDeepPartialType(Inner).What is the new behavior?
The plugin fields branch now applies
ApiPropertyeagerly with the same recursive type substitution used for explicitly decorated fields. This is the pattern #3822 introduced forPartialType,OmitTypeandPickType.DeepPartialTypelanded at almost the same time as that PR, so the two were in flight together and it never picked the pattern up.isDtoClassalso falls back to detecting the metadata factory, which is what actually enables the recursive wrapping. Reverting only that part while keeping the plugin fields branch still fails three of the new tests, and all three are the nested wrapping cases. Both changes are load bearing.The fallback also fixes a second case that has nothing to do with the plugin branch: an explicit
@ApiProperty({ type: () => SomePluginDto })pointing at a plugin generated DTO now gets wrapped too.While adding the fallback I hit a crash that already exists on master. If a lazy type factory throws, for example during a circular import, the catch hands the arrow function itself back as the resolved type. Arrow functions have no prototype, so
getModelPropertiesdereferencesundefinedand reflect-metadata throws out ofDeepPartialTypeat class definition time. That path was reachable through explicit decorators before this PR, and the plugin branch would have widened it, since the plugin emitstype: () => Xfor every property. A prototype guard inisDtoClasscloses both.Tests cover the plugin only nested DTO, deep recursion through two levels, plugin declared arrays, the mixed case where one class has both explicit and plugin metadata, and the throwing factory on both routes. The existing explicit decorator tests are unchanged and keep passing, which is the control.
Does this PR introduce a breaking change?
Other information
Full suite passes at 412 tests. Reverting the source while keeping the new tests fails 5 of them.