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

Look into options for handling reactive-related methods firing before the DOM is mounted #3011

Closed
davep opened this issue Jul 25, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request Task wontfix This will not be worked on

Comments

@davep
Copy link
Contributor

davep commented Jul 25, 2023

There is a class of problem, that can be worked around by someone developing with Textual, where there is a var or reactive that has watch or other similar methods tied to it, that in turn try and do work that needs that the DOM is fully-mounted; but then it's possible for the developer to modify the value of the var/reactive before mounting has happened.

This inevitably results in DOM-related errors.

This can be best seen in #2363 and the PR that sought to fix it (#3002).

In conversation we've decided that it would be a good idea to look for a more general way of solving this problem. While DirectoryTree, or even Tree, could be modified to not have this be an issue, the fact remains that anyone building their own widgets will run into the same problem.

Current initial thinking is that perhaps var and reactive should grow some sort of post_mount keyword flag, which when True would tell Textual to delay any watch or other similar calls until after the DOM is mounted.

@davep davep added enhancement New feature or request Task labels Jul 25, 2023
@darrenburns darrenburns self-assigned this Oct 25, 2023
@darrenburns
Copy link
Member

We've opted against post_mount, but I'm out of ideas for a generic approach that is clean.

Accessing the DOM or calling get_component_styles inside a watcher will currently result in a crash, if a user sets the corresponding reactive before mounting it.

Closely related to #2363 which is caused by this.

@willmcgugan I'm blocked on this at the moment.

@rodrigogiraoserrao
Copy link
Contributor

We don't want to skip or delay calling watchers until the Mount event because that would probably be confusing: the user sets a reactive, expects the remainder of the widget state to update (as per the logic they might implement in the watcher) but then the watcher doesn't run because the widget hasn't been mounted yet.

The “solution” seems to be avoiding making use of the DOM in watchers and/or checking if the widget has been mounted with is_mounted.

Copy link

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
enhancement New feature or request Task wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants