Skip to content

Conversation

Umpire2018
Copy link

Description

At first glance, other modules works fine. e.g. https://docs.litestar.dev/main/reference/middleware/logging.html#litestar.middleware.logging.LoggingMiddlewareConfig

Image

After digging in, i found that if a class is imported into __init__.py via from ... import ... and added to __all__, Sphinx may lose the original source order of its members so i changed to document classes directly from their original module, rather than through indirect imports in __init__.py.

Before After
image image

Closes

close #4393

@Umpire2018 Umpire2018 requested review from a team as code owners October 14, 2025 09:23
@github-actions github-actions bot added area/docs This PR involves changes to the documentation size: small type/bug pr/external Triage Required 🏥 This requires triage labels Oct 14, 2025
@Umpire2018 Umpire2018 force-pushed the Umpire2018/issue4393 branch from 3865d12 to 863a38d Compare October 14, 2025 09:31
Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/4436

@provinzkraut
Copy link
Member

Interesting find! Does this replicate in a fresh project? If so, I think you should submit this as a bug to https://github.com/sphinx-doc/sphinx/.

As for the proposed fix on our side, I'm not entirely sure this is fine to do, since it would mean that only members of litestar.middleware.base get documented, and things that don't originate there but are imported into litestar.middleware are not. We could of course explicitly document litestar.middleware.base, but then I think it would be confusing if the import is from litestar.middleware import ASGIMiddleware, but the docs are for litestar.middleware.base.ASGIMiddleware.

@euri10, got some thoughts on this? I know how much you like to consult documentation :)

@euri10
Copy link
Contributor

euri10 commented Oct 14, 2025

vey interesting finding yeah.

i agree with you it would be confusing, and if i had to choose i'd sacrifice the order for the "correct" import.

now the order is already weird to me in other cases, not just class member, if you look at https://docs.litestar.dev/main/reference/middleware/constraints.html for instance i find it weird to get exceptions before classes for instance,

- Fixes issue where class members are sometimes listed alphabetically instead of by source in middleware documentation.(litestar-org#4393)
- Updates references to use the base module.
@provinzkraut provinzkraut enabled auto-merge (squash) October 14, 2025 19:41
@provinzkraut
Copy link
Member

Alright, then let's merge this and see if can can sort out the imports again once the bug has been fixed upstream.

@euri10
Copy link
Contributor

euri10 commented Oct 15, 2025

Alright, then let's merge this and see if can can sort out the imports again once the bug has been fixed upstream.

any issue to follow ?

@Umpire2018
Copy link
Author

Umpire2018 commented Oct 15, 2025

I think it would be confusing if the import is from litestar.middleware import ASGIMiddleware, but the docs are for litestar.middleware.base.ASGIMiddleware.

  1. I agree with you on that. After investigating more on that, i think it may be better to leave it to what it is nowadays because i don't think upstream will fix this autodoc related issue considering there are lots of ongoing tickets right now.

  2. It seems like class members are in alphabetical sequence elsewhere instead of by source. See here.

image

Maybe we can consider alternatives like https://docusaurus.io/ ?

@euri10
Copy link
Contributor

euri10 commented Oct 15, 2025

idk https://docusaurus.io/ to be fair so I cant comment on this
I had good success with the autoapi extension but I have to admitt I didnt dwelve into all the details, but overall I found it to be easier to use and have a nicer look than autodoc, but that is just a very uninformed impression

@Umpire2018
Copy link
Author

now the order is already weird to me in other cases, not just class member, if you look at https://docs.litestar.dev/main/reference/middleware/constraints.html for instance i find it weird to get exceptions before classes for instance,

@euri10 Maybe this is expected behavior ? Because this is the sequence in source code and this PR haven't introduce relevant changes.

class MiddlewareConstraintError(LitestarException):
pass
class ConstraintViolationError(MiddlewareConstraintError):
pass
class CycleError(MiddlewareConstraintError):
pass
@dataclasses.dataclass(frozen=True)
class MiddlewareForwardRef:

@euri10
Copy link
Contributor

euri10 commented Oct 15, 2025

now the order is already weird to me in other cases, not just class member, if you look at https://docs.litestar.dev/main/reference/middleware/constraints.html for instance i find it weird to get exceptions before classes for instance,

@euri10 Maybe this is expected behavior ? Because this is the sequence in source code and this PR haven't introduce relevant changes.

class MiddlewareConstraintError(LitestarException):
pass
class ConstraintViolationError(MiddlewareConstraintError):
pass
class CycleError(MiddlewareConstraintError):
pass
@dataclasses.dataclass(frozen=True)
class MiddlewareForwardRef:

yep sorry for the confusion i was speaking in general, its expected behaviour as you rightully say, with autoapi this would be ordered differently I think without thinking about how it's organized in source,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs This PR involves changes to the documentation pr/external pr/internal size: small Triage Required 🏥 This requires triage type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs: Member order of classes is sometimes alphabetical instead of by source

3 participants