Skip to content

Add type annotations #877

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add type annotations #877

wants to merge 2 commits into from

Conversation

ViktorSky
Copy link

This pull request introduces comprehensive type annotations to the firebase_admin codebase using PEP 484 standards. All changes have been validated with Pyright in strict mode to enforce consistency and catch potential type errors early.

Goals

  • Enhance developer experience with precise autocomplete and type hints.
  • Catch type-related bugs upfront via static analysis.

Tools

  • Pyright in strict mode for robust type-checking.
  • Manual review of ambiguous or potentially problematic sections.

Configuration Additions

  • pyrightconfig.json: config file created to declare strict-mode settings.

  • firebase_admin/_typing.py: A new central module that defines all custom type aliases, TypedDicts, and Protocols. By consolidating these shared types in one place, we simplify extension and reuse across the package and avoid circular-import issues.

@ViktorSky
Copy link
Author

ViktorSky commented May 20, 2025

Before approving this request, I must warn you that I cannot use pylint, because the configured version of astroid can only be used on versions lower than py3.11.
I also left some comments in the code about potential runtime issues.

  • firebase_admin._messaging_encoder._Validators
    Why not @staticmethod? Perhaps there were past issues or subclassing considerations?

  • firebase_admin.remote_config._ConditionEvaluator.evaluate_conditions
    You probably need to change condition.get('name') to condition['name']

  • firebase_admin.remote_config._ConditionEvaluator._compare_semantic_versions
    bad docstring

  • firebase_admin.remote_config._ConditionEvaluator.evaluate
    parameter_value_wrapper.get('value') is only valid if None is a valid argument to _Value

  • firebase_admin.functions.TaskQueue._update_task_payload
    service_account_email is accessed on a type that might not have that attribute

@ViktorSky
Copy link
Author

To upload to pypi you must approve #713, because linters like mypy will ignore it since it is not marked as safe

@ViktorSky ViktorSky mentioned this pull request May 21, 2025
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.

1 participant