Skip to content
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

Tidy the return type of App.render #4063

Closed
davep opened this issue Jan 23, 2024 · 3 comments · Fixed by #4264
Closed

Tidy the return type of App.render #4063

davep opened this issue Jan 23, 2024 · 3 comments · Fixed by #4264
Assignees
Labels
bug Something isn't working enhancement New feature or request Task

Comments

@davep
Copy link
Contributor

davep commented Jan 23, 2024

Currently the return type of App.render is RenderableType; in our docs and in most places we talk about App.render returning a RenderResult. RenderResult is an alias for RenderableType and is a top-level value in textual.app.

This can cause a lint/type warning in dev's apps if they let their IDE complete the signature when writing their own render method, as it will often complete to something like this:

    def render(self) -> RenderableType:
        return super().render()

which in turn will auto-import RenderableType from textual.app. This can then result in a warning like this:

Screenshot 2024-01-23 at 14 16 46

We should change App.render so that it's typed as returning RenderResult rather than RenderableType, then such completion will use the type that is exported from textual.app.

Also note that the code in the compose/render HOWTO seems to have fallen foul of this and has that type warning.

@davep davep added bug Something isn't working enhancement New feature or request Task labels Jan 23, 2024
@TomJGooding
Copy link
Contributor

TomJGooding commented Jan 24, 2024

This seems a simple enough fix that I could help (if wanted!). But I can't seem reproduce this in my editor, so I just wanted to check where you've seen/tested this?

[EDIT: I'm assuming certainly in VSCode from other instances I found. I've gone ahead with a PR but needs a double-check that this is indeed the fix!]

@davep
Copy link
Contributor Author

davep commented Jan 25, 2024

But I can't seem reproduce this in my editor, so I just wanted to check where you've seen/tested this?

In my case Emacs+eglot+pyright while auto-completing def render.

@davep davep self-assigned this Mar 5, 2024
davep added a commit to davep/textual that referenced this issue Mar 6, 2024
Use the type hint that is talked about in the documentation.

Fixes Textualize#4063.
Copy link

github-actions bot commented Mar 6, 2024

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request Task
Projects
None yet
2 participants