Skip to content

fixed types annotations for meta in add_page() to accept components #6012

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.

Sign up for GitHub

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

Jump to bottom
Merged
masenf merged 3 commits into from
Dec 2, 2025
Merged

fixed types annotations for meta in add_page() to accept components #6012

masenf merged 3 commits into from
Dec 2, 2025
+7 −7

Conversation

@dennisbakhuis
Copy link
Contributor

@dennisbakhuis dennisbakhuis commented Nov 30, 2025

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully ran tests with your changes locally?

Description

Updated type annotations for the meta parameter to accept Component in addition to dict[str, str], making the typing more accurate and flexible.

Previous PR added support for adding any components as meta to add_page() but the type has was not changed. This PR modifies the changes.

Changes:

  • reflex/app.py: Updated meta type from Sequence[Mapping[str, str]] to Sequence[Mapping[str, str] | Component] in UnevaluatedPage dataclass and add_page() method
  • reflex/compiler/utils.py: Updated meta parameter type in add_meta() function from list[dict] to list[Mapping[str, Any] | Component]
  • tests/units/test_page.py: Updated type of meta to the actual type, which is dict by default.

Why:

The existing type annotations were too restrictive. The meta parameter can accept Component objects (for link components, script tags, etc.), not just dictionaries. This change aligns the type hints with the actual runtime behavior.

Personally, my OCRs triggered when meta links in add_page() triggered a type error. It is a small PR for some peace of mind. Thanks for the fantastic framework. I am having a lot of fun with it.

Testing:

  • passed local type checking passed (pyright)
  • passed unit tests passed locally
  • passed code linted and formatted (ruff)

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 30, 2025

Greptile Overview

Greptile Summary

Updated type annotations for the meta parameter in add_page() to accurately reflect that it accepts both Mapping[str, Any] and Component objects, aligning the type hints with the existing runtime behavior.

  • Changed meta type from Sequence[Mapping[str, str]] to Sequence[Mapping[str, Any] | Component] in UnevaluatedPage dataclass and add_page() method
  • Updated add_meta() function signature to match the new type definition
  • Corrected test data to use proper dict structure instead of invalid string value

The changes properly align type annotations with the implementation in reflex/compiler/utils.py:581 which already handles both Component objects and dicts via isinstance() check.

Confidence Score: 5/5

  • This PR is safe to merge with no concerns
  • The changes are purely type annotation updates that make the static type hints match the actual runtime behavior. The implementation already handled both Component and dict types correctly via isinstance checks. No logic changes, no breaking changes, and the test correction improves test accuracy.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
reflex/app.py 5/5 Updated type annotations for meta parameter in UnevaluatedPage and add_page() to accept Component objects in addition to mappings
reflex/compiler/utils.py 5/5 Updated add_meta() function signature to accept `Sequence[Mapping[str, Any]
tests/units/test_page.py 5/5 Corrected test data from invalid string \"foo-meta\" to proper dict structure matching actual meta format

Sequence Diagram

sequenceDiagram
    participant User
    participant App
    participant UnevaluatedPage
    participant Compiler
    participant add_meta

    User->>App: add_page(meta=[dict | Component])
    App->>UnevaluatedPage: Create with meta: Sequence[Mapping[str, Any] | Component]
    UnevaluatedPage->>Compiler: Pass page with meta
    Compiler->>add_meta: add_meta(meta: Sequence[Mapping[str, Any] | Component])
    add_meta->>add_meta: For each item: if Component use as-is, else Meta.create(**item)
    add_meta->>Compiler: Return component with meta tags
    Compiler->>User: Rendered page with metadata
Loading

greptile-apps[bot]
greptile-apps bot reviewed Nov 30, 2025
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 2, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #6012 will not alter performance

Comparing dennisbakhuis:page_meta_link_types (1e414a3) with main (a78f358)

Summary

✅ 8 untouched

masenf
masenf approved these changes Dec 2, 2025
View reviewed changes
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

thank you for your contribution ☺️

@masenf masenf merged commit e4300ad into reflex-dev:main Dec 2, 2025
44 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@masenf masenf masenf approved these changes

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dennisbakhuis @masenf