feat(types): TagifiedTagList/TagifiedTag as real subclasses (#116)#118
Closed
schloerke wants to merge 26 commits into
Closed
feat(types): TagifiedTagList/TagifiedTag as real subclasses (#116)#118schloerke wants to merge 26 commits into
schloerke wants to merge 26 commits into
Conversation
Replaces the TagifiedTagList TypeAliasType with a real subclass and adds a parallel TagifiedTag. Subclasses are runtime-isinstance-checkable. Also adds a non-generic TagifiedChild alias parallel to TagChild. Mutator overrides and the .tagify() return-type updates come in follow-up commits.
…#116) The cast-over-copy() pattern returned a base TagList at runtime; now that TagifiedTagList is a real subclass, we construct one explicitly so isinstance(result, TagifiedTagList) is True.
The previous comment cited .data's runtime contents; the cast is actually about pyright's static narrowing under the TagifiedTagList return type. The runtime guard exists to catch user .tagify() protocol violations.
…#116) TagifiedTag (subclass of Tag) was failing equality against bare Tag because isinstance(y, type(x)) is False when x is the subclass and y is the parent. The subclass adds no instance attributes, so two objects with matching __dict__s should compare equal regardless of which is the subclass. Fixes test_taglist_tagifiable regression.
Overrides on __init__/append/extend/insert reject un-tagified inputs at the pyright call site. Pass-through bodies — no new runtime guards; existing tagify-boundary and render-time guards remain the runtime safety net. Subsumes the static-enforcement work of #115.
…ifiedTagList (#116) Annotations are already lazy via __future__ import; quoted strings on override signatures were inconsistent with the parent TagList. Also adds a docstring paragraph explaining why the reportIncompatibleMethodOverride suppressions are deliberate.
The previous test documented a static-typing gap that #116 closes. Its body now asserts the static rejection that the subclass overrides provide. Also updates the line-27 assert_type to reflect Tag.tagify()'s new return type TagifiedTag.
…#116) Originally planned as a fixture for the variance ERROR that the subclass approach was expected to surface. Empirical check: pyright accepts TagifiedTagList -> TagList (bare AND explicit-parameter) and the same for TagifiedTag -> Tag. Likely due to nominal-subclass handling + TypeVar default precedence. The fixture flips to lock in the observed *permissive* behavior — if pyright ever changes, this catches the regression.
The trailing two sentences claiming that TagList[TagifiedNode].append "no longer static-errors" became false once #116 added narrow-signature mutator overrides on TagifiedTagList / TagifiedTag. The new comment states the static-input enforcement story correctly.
Adds entries for the TagifiedTagList -> class, TagifiedTag, and TagifiedChild additions. Calls out that pyright remains permissive on TagifiedTagList -> bare TagList flows in practice, so downstream consumers mostly don't need migration. Also retires the now-false "mutation methods still accept Tagifiable statically" note from #105 since #116 added the narrow-signature overrides.
There was a problem hiding this comment.
Pull request overview
This PR turns TagifiedTagList (and new TagifiedTag) into real runtime subclasses returned by .tagify(), enabling isinstance checks and allowing narrower mutator signatures so pyright can reject un-tagified inputs to tagified containers.
Changes:
- Replace the
TagifiedTagList = TypeAliasType(...)alias withclass TagifiedTagList(TagList["TagifiedNode"]), and addclass TagifiedTag(Tag["TagifiedNode"]). - Update
TagList.tagify(),Tag.tagify(), andJSXTag.tagify()to construct/return these subclasses at runtime. - Add
TagifiedChildand narrow mutator overrides (__init__/append/extend/insert) for static input enforcement; adjust equality to be subclass-symmetric; add runtime tests and expanded pyright fixture tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| htmltools/_core.py | Defines TagifiedChild, TagifiedTagList, TagifiedTag, updates tagify() to return real subclasses, and adjusts equality symmetry. |
| htmltools/_jsx.py | Switches JSXTag.tagify() to return a TagifiedTag constructed directly. |
| htmltools/init.py | Exports TagifiedChild, TagifiedTag, and TagifiedTagList as public API. |
| tests/test_types.py | Updates/extends pyright fixture tests for the new subclass types and narrowed mutator signatures. |
| tests/test_tagified_subclasses.py | Adds runtime isinstance/type(...) is ... assertions for the new subclasses and .tagify() returns. |
| CHANGELOG.md | Documents the new subclasses, narrowed mutator typing, and migration implications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
| ) | ||
| else: | ||
| cp[i] = tagified_child |
…fy() returns Two paired changes that only make sense together: 1. Collapse `TagifiedChild` into `Tagified`. `Tagified` is now the single non-generic union covering both `Tagifiable.tagify()`'s return shape AND the input-side type for mutators on `TagifiedTagList` / `TagifiedTag`. Includes the same flattening conveniences as `TagChild` (`float`, `None`, `Sequence[Tagified]`). Drops `TagifiedTag`, `TagifiedTagList`, and `TagifiedChild` from the top-level `htmltools` namespace — the classes stay accessible via `htmltools._core` for code that wants to isinstance-check, but the public surface added by this PR is just the widened `Tagified`. 2. Fix #117: `TagList.tagify()` now routes every return from a child's `.tagify()` through `_tagchilds_to_tagnodes`. That uniformly handles `None` (dropped), `float`/`int` (str-ified), `Sequence` (flattened), `TagList` (flattened), and plain nodes (passthrough). Previously only the `TagList` case was flattened; the other shapes slipped past the boundary check and either crashed the render path (`None` → `TypeError` in `html_escape`) or silently corrupted the tag tree. Without (2), the widening in (1) would create a static/runtime gap: pyright would accept `def tagify(self) -> Tagified: return None` while the framework still mishandled the return. Together, the static type and the runtime contract agree. Closes #117.
Eight entries -> five. The two `Tagified`-shape entries are now one; the two `TagifiedTagList` subclass entries (subclass-ness + mutator narrowing) are merged with sub-paragraphs; the two boundary-check bug fixes are combined into one entry covering the full normalization.
The previous post-condition accepted any `Tag`/`TagList` in the tagified result — but `Tagified` excludes bare `Tag`/`TagList` (only `TagifiedTag` and `TagifiedTagList` are in the contract). A misbehaving `.tagify()` returning `div(some_tagifiable)` (a bare Tag wrapping an un-tagified child) slipped past the boundary and only failed at render time with the misleading "tree was mutated to add a Tagifiable" message. Tightens the check to require `TagifiedTag`/`TagifiedTagList` specifically. Updates the in-tree DelayedDep test that was relying on the lenient behavior — fix is a one-line `.tagify()` on the return. Adds three new tests covering bare-Tag returns with both Tagifiable and leaf children. Addresses Copilot review comment on PR #118.
…Child The wider union now has two names in `_core.py`: - `TagifiedChild` (internal): the input-side union, named to parallel `TagChild` so maintainers can see the structural symmetry between the un-tagified and tagified sides. - `Tagified` (public): the user-facing name for the same shape, used in `Tagifiable.tagify()`'s protocol return type. Aliased via `Tagified = TagifiedChild`. Override signatures inside `TagifiedTagList` / `TagifiedTag` switch to `TagifiedChild` to reinforce the input-side parallel. Public-facing docstrings and the CHANGELOG continue to refer to `Tagified` (still the only exported name). At runtime the two names refer to the same Union object.
- Removes the "subclasses defined later in this file" preamble above the alias block; the surrounding code is now self-evident. - Moves the "Note on overrides" LSP-narrowing paragraph from each subclass docstring into a leading `#` comment block. It's maintainer context, not user-facing API documentation.
Adds `_LSPNarrowingCanary` to tests/test_types.py that replicates the contravariant-narrowing override pattern from `TagifiedTagList` / `TagifiedTag` in `_core.py`, with the same `# pyright: ignore[reportIncompatibleMethodOverride]` suppressions. File-level pyright config enables `reportUnnecessaryTypeIgnoreComment= error` and `reportIncompatibleMethodOverride=error`. The first makes all `# pyright: ignore` comments in the file self-validating; the second is needed because the rule defaults to "none" in basic mode when checking cross-module overrides. Together they form a tripwire: if pyright ever stops considering input-narrowing an LSP violation, the canary's `# pyright: ignore` becomes unused and CI fails — that's the signal to revisit the suppression in `_core.py` (and possibly switch to the `TagChild[TagNodeT]` parameterized-parent approach from #105). Also retires a related stale `# pyright: ignore[reportAssignmentType]` in `test_user_tagify_returning_bare_TagList_violates_Tagifiable`: the widening of `Tagified` to include `Sequence[Tagified]` made `TagList` structurally compatible with the `Tagifiable` protocol (`TagList` is a `Sequence`), so the assignment no longer errors. Renamed the test to `..._is_structurally_Tagifiable` and updated its docstring to document the observed leniency.
5 tasks
Collaborator
Author
|
Closing in favor of #120 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TagifiedTagList/TagifiedTagare real subclasses now. ReplacesTagifiedTagList = TypeAliasType(...)withclass TagifiedTagList(TagList["TagifiedNode"]). Adds parallelclass TagifiedTag(Tag["TagifiedNode"]). Both are runtime-isinstance-checkable and are constructed (not cast) byTagList.tagify()/Tag.tagify()/JSXTag.tagify(). The classes stay internal tohtmltools._core— they are NOT re-exported from the top-levelhtmltoolsnamespace. Code that wants toisinstance-check imports them explicitly fromhtmltools._core.__init__/append/extend/inserttakeTagified(a non-generic union that excludes the un-resolvedTagifiablearm ofTagChild). Pyright rejects un-tagified inputs to tagified containers at the call site. Subsumes Static input enforcement onTagList[TagifiedNode]viaself-typed overloads #115 — the parallel session exploringSelf-typed overloads was abandoned because pyright couldn't make that approach work; this PR closes the same static-input gap via the subclass overrides instead.Tagifiedcollapses what was previouslyTagified+TagifiedChildinto one alias. Now mirrorsTagChild's shape (excludesTagifiable, includes the flattening conveniencesfloat,None,Sequence[Tagified]). Custom.tagify()implementations may return any of those shapes.TagList.tagify()now routes every child's.tagify()return through_tagchilds_to_tagnodes, uniformly handlingNone(dropped),float/int(str-ified),Sequence(flattened),TagList(flattened), and plain nodes (passthrough). Previously only theTagListcase was flattened; the other shapes slipped past the boundary check and crashed the render path (None→TypeErrordeep inhtml_escape) or silently corrupted the tag tree. Without this, the widenedTagifiedstatic contract above would create a static/runtime gap._equals_implis now subclass-symmetric soTagifiedTag == Tag(andTagifiedTagList == TagList) compares structurally when__dict__s match. Caught a regression intest_taglist_tagifiablethat surfaced onceTag.tagify()started returning the subclass.Closes #116. Closes #115 (subsumed). Closes #117.
Public surface added by this PR
Just the widened semantics of
Tagified(already an exported name pre-#116; now includesfloat/None/Sequence[Tagified]and serves as both the.tagify()return type and the input type for tagified-container mutators). The new classesTagifiedTag,TagifiedTagList, and the recursive helper aliasTagifiedNodestay internal tohtmltools._core. The normal authoring path (def tagify(self) -> Tagified: ...) only needs the one alias.Migration impact (smaller than feared)
The "Open question" in #116 — whether the alias was silently relaxing variance — has an empirical answer in this PR's fixture tests: pyright accepts
TagifiedTagList → TagList(bare andTagList[TagNode]) andTagifiedTag → Tagthanks to nominal-subclass +TypeVar(default=TagNode)handling. So downstream consumers of.tagify()mostly do NOT need migration. A test intests/test_types.pylocks in this observed permissive behavior so a future pyright change that flips it is caught.The narrow-signature overrides DO surface intentional static errors at one place: appending an un-tagified
Tagifiableto aTagifiedTagList/TagifiedTag. That's the deliberate gap-closing. Users who need to bypass it can# pyright: ignoreand rely on the existing tagify-boundaryTypeError/ render-timeRuntimeErroras the runtime safety net.Test plan
make checkis clean — ruff format + pyright (0 errors) + pytest (106 passing across 11 test files)TagList("hi", div()).tagify()returns aTagifiedTagList, itsTagchildren areTagifiedTaginstances, full tree renders to HTMLTagifiableinputs toTagifiedTagList.append/extend/insertand toTagifiedTag.append/extend/insertTagifiedTagList → TagList,TagifiedTag → Tag) — if pyright ever flips, the fixture catches it.tagify()returningNone/float/Sequenceis normalized at the boundary)_equals_implsubclass symmetry verified by the existingtest_taglist_tagifiable(now passing)