-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] Use TypeInfo for #[MapRequestPayload]
type resolution
#58036
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
base: 7.4
Are you sure you want to change the base?
Conversation
45b01c9
to
7cba23b
Compare
7cba23b
to
0643d9a
Compare
if ($this->typeResolver) { | ||
$type = $this->typeResolver->resolve($parameter); | ||
|
||
if ($type instanceof CollectionType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to "unwrap" a potential nullable type here I think
+ /**
+ * @param Product[] $products
+ */
#[Route(path: "/products", methods: "POST")]
public function __invoke(
- #[MapRequestPayload(type: Product::class)] array $products,
+ #[MapRequestPayload] array $products,
): Response {
} Honestly, when you mentioned this feature during the |
I get your concern, yet that phpdoc is needed anyway for type detection in IDEs/static analyzers to work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense but IMHO:
- I don't think the deprecation is needed nor desired
- This adds advanced type resolution to the hot-path, while this info might not be used; we should look for a way to make this parsing lazy
Not strictly needed indeed but: it's not obvious what
👍 I will wrap that logic in a closure and pass it to |
Still not convinced about the deprecation :) |
Simplifying mapping request parameters to object lists. Before/After:
/cc @yceruto @mtarld