Skip to content

fix(typing): give Application.include_router a fully-known signature#181

Open
tmgbedu wants to merge 1 commit into
mainfrom
task/router-include-router-typing
Open

fix(typing): give Application.include_router a fully-known signature#181
tmgbedu wants to merge 1 commit into
mainfrom
task/router-include-router-typing

Conversation

@tmgbedu

@tmgbedu tmgbedu commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • basedpyright's reportUnknownMemberType fired on include_router because Application.include_router (fastapi_startkit/src/fastapi_startkit/application.py) took untyped **kwargs and had no return annotation, so pyright inferred a partially-Unknown signature for every call site.
  • Investigated whether this was a FastAPI stub gap: it isn't — FastAPI.include_router() itself is fully typed with explicit named parameters. The leak was entirely in our own wrapper method.
  • Fixed by giving include_router a complete, explicit signature: a new IncludeRouterOptions TypedDict mirrors FastAPI's real include_router() keyword parameters (mirrors the existing RouterOptions/RouteOptions pattern already used in fastapi/routers/router.py), consumed via Unpack[IncludeRouterOptions], plus an explicit "Application[TConfig]" return type.
  • Kept the framework's lazy-FastAPI-import contract intact: all fastapi/starlette-only types in the new TypedDict are referenced as quoted forward refs (matching the file's existing "APIRouter"/Type["BaseHTTPMiddleware"] style), so fastapi_startkit.application still imports fine without the fastapi extra installed.

Test plan

  • uv run --with basedpyright basedpyright against a strict pyright config no longer reports any include_router diagnostic in application.py (confirmed 0 matches, down from the reported reportUnknownMemberType)
  • uv run pyright (project's own basic-mode config used in CI) — error count unchanged (659, pre-existing unrelated baseline), zero errors in application.py before and after
  • uv run ruff check . and uv run ruff format --check — clean
  • uv run pytest tests/ -q --ignore=tests/masoniteorm/postgres — 1833 passed, 7 skipped

reportUnknownMemberType fired at every include_router call site because
the method's **kwargs and return type were unannotated, so pyright/
basedpyright inferred an "Unknown"-tainted signature for it. The
underlying FastAPI.include_router() itself is fully typed, so the leak
was entirely in our own wrapper.

Type the accepted kwargs with an IncludeRouterOptions TypedDict mirroring
FastAPI's own include_router() parameters (mirrors the existing
RouterOptions/RouteOptions pattern in fastapi/routers/router.py) and add
an explicit "Application[TConfig]" return annotation. Types that require
fastapi/starlette are referenced as quoted forward refs, consistent with
the rest of this file, so importing fastapi_startkit.application still
does not require the fastapi extra to be installed.
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@tmgbedu tmgbedu left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Code Review Verdict: APPROVE ✅ (posting as comment — GH blocks self-approval on own PR)

Reviewed for correctness, typing soundness, lazy-import contract, and regressions. All engineer claims verified independently.

Verification performed:

  1. TypedDict fields match FastAPI — all 9 fields (prefix, tags, dependencies, responses, deprecated, include_in_schema, default_response_class, callbacks, generate_unique_id_function) match FastAPI.include_router() in the installed FastAPI 0.136.3 exactly, correct types, no missing/extra fields.
  2. Lazy-import contract preserved — empirically created the TypedDict + Unpack[...] signature with fastapi/starlette imports blocked: succeeds, fastapi never enters sys.modules, FastAPI/Starlette-typed fields stay unevaluated ForwardRefs. params.Depends/Response/BaseRoute/APIRoute correctly quoted; only stdlib/Enum types unquoted.
  3. No suppressions — no # type: ignore / # pyright: ignore / # noqa. Only Any is the legitimate Dict[str, Any] mirroring FastAPI's responses.
  4. CI checks hold — ruff check + format clean; uv run pyright (basic mode) total unchanged at 659, application.py basic-mode errors identical main vs PR (all pre-existing None-default / Optional[_fastapi], unrelated); pytest 1833 passed, 7 skipped (Postgres integration failures are environmental — fail identically on main, missing DB creds). CI pyright step is continue-on-error: true / non-blocking.
  5. Scope clean — only application.py touched (+23/-3), all new imports used, mirrors the existing RouterOptions/RouteOptions TypedDict+Unpack idiom in router.py.

Not merging per instructions — PM to merge.

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