Skip to content

Keep the sub-app's deprecated flag in add_typer() - #1934

Open
VenishPaneliya wants to merge 1 commit into
fastapi:masterfrom
VenishPaneliya:fix/add-typer-deprecated-default
Open

Keep the sub-app's deprecated flag in add_typer()#1934
VenishPaneliya wants to merge 1 commit into
fastapi:masterfrom
VenishPaneliya:fix/add-typer-deprecated-default

Conversation

@VenishPaneliya

Copy link
Copy Markdown

Typer.add_typer() declares its deprecated parameter as a plain False, while every other setting in the same signature uses the Default() sentinel — including hidden, which sits directly above it:

        hidden: Annotated[...] = Default(False),
        deprecated: Annotated[...] = False,          # <- the odd one out

solve_typer_info_defaults() uses that sentinel to tell "the caller passed this to add_typer()" apart from "the caller said nothing":

        # Priority 1: Value was set in app.add_typer()
        if not isinstance(value, DefaultPlaceholder):
            values[name] = value
            continue
        # Priority 2: Value was set in @subapp.callback()
        ...
        # Priority 3: Value set in subapp = typer.Typer()

A bare False is not a DefaultPlaceholder, so Priority 1 always fires and Priorities 2 and 3 are never reached. A sub-app that marked itself deprecated silently loses that flag the moment it is added to a parent.

Reproduction

import typer

app = typer.Typer()
sub_app = typer.Typer(deprecated=True)

@sub_app.command()
def sub_command():
    """Sub command."""

app.add_typer(sub_app, name="sub")

--help on the parent:

│ sub                                                                          │

expected:

│ sub                                                            (deprecated)  │

Why this is the default and not a judgement call

Two controls, same shape, both behave correctly:

Case Result
sub_app = typer.Typer(hidden=True) — sibling, Default()-wrapped ✅ propagates
app.add_typer(sub_app, deprecated=True) — passed at the call site ✅ shows (deprecated)
sub_app = typer.Typer(deprecated=True) — this bug ❌ silently dropped

So the plumbing is fine end to end; only the default is wrong.

TyperInfo is constructed in exactly three places, and all three flow through solve_typer_info_defaults():

deprecated default
Typer.__init__ Default(False)
Typer.callback() Default(False)
Typer.add_typer() False

Typer.command() also uses a bare False, but that one builds a CommandInfo, which never goes through the solver — so it is correctly different and is left alone here.

Change

One token: ] = False,] = Default(False),. Default is already imported in the module.

Tests

Added test_add_typer_keeps_sub_app_deprecated to tests/test_deprecation.py, parametrised over rich / non-rich the same way tests/test_hidden.py does, since the marker renders as (deprecated) under Rich and (DEPRECATED) without it.

  • Against master: both parametrisations fail
  • With the change: pass
  • Full suite before: 1379 passed, 18 skipped, 2 xfailed
  • Full suite after: 1381 passed, 18 skipped, 2 xfailed — no other test changes behaviour
  • ruff check and ruff format --check clean on both changed files
  • mypy typer reports the same 6 pre-existing errors before and after (identical set); none in main.py

add_typer() declared `deprecated` as a plain False while every other
setting in the same signature uses the Default() sentinel, including
`hidden` on the line above it.

solve_typer_info_defaults() relies on that sentinel to tell "the caller
passed this to add_typer()" apart from "the caller said nothing". A bare
False is not a DefaultPlaceholder, so Priority 1 always matched and the
callback and sub-app instance were never consulted: a sub-app that marked
itself deprecated lost the flag as soon as it was added to a parent.

Passing deprecated=True to add_typer() directly already worked, and the
sibling `hidden` propagates correctly, so only the default was wrong.

Typer.command() also uses a bare False, but it builds a CommandInfo, which
never goes through solve_typer_info_defaults(), so it is left as is.
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