Skip to content

feat: validate component inputs if types are given #629

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

Merged

Conversation

JuroOravec
Copy link
Collaborator

@JuroOravec JuroOravec commented Aug 29, 2024

As I mention in #622 (comment), this MR adds validation of args, kwargs, slots, and data (returned from get_context_data) based on the types passed to the Component's generics.

The reason I wanted to add this is following:

When I was adding the typing support to the components, I had two options:

  1. Either use ParamSpec as one of the Component's generics, which would be bound to the get_context_data. This way, the get_context_data function would have been the source of truth for the type.
  2. Or define args and kwargs as I eventually did, as a Tuple and TypedDict. This way, the Component.render would still be correctly typed, but the signature of get_context_data could diverge.

I went with option 2, because even I haven't worked with ParamSpec before, and I didn't want to ask users to pass around something which they might have never used before. It seemed complex. On the other hand, defining just tuples and typed dicts seems a lot more intuitive.

But that means that one could add typing for the Component, and the args and kwargs declared in the types could be totally different from the actual implementation of get_context_data.

So this MR adds some guard rails so that the component's implementation must be compatible with the declared args and kwargs types.

Altho, this is applicable only for Python 3.11 and later. In older versions, args are still validated, but TypedDicts like kwargs, slots, and data, are typed only, but not validated.

@@ -632,58 +634,6 @@ MyComponent.render_to_response(
)
```

### Adding type hints with Generics
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved this subsection to its own section on typing and validation.

Copy link
Collaborator

@EmilStenstrom EmilStenstrom left a comment

Choose a reason for hiding this comment

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

This is way above my head, so can't review. I trust it if the tests pass, so go ahead! :)

@JuroOravec
Copy link
Collaborator Author

@EmilStenstrom Appreciate the honesty. I can be easily too deep in things, so I've added more comments to the code, hopefully making it clearer!

@JuroOravec JuroOravec merged commit 4a9cf7e into django-components:master Aug 29, 2024
7 checks passed
@JuroOravec JuroOravec deleted the jo-feat-input-validation branch August 29, 2024 21:09
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