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

Current App Id is Updated before Previous App is Unmounted #82877

Open
ThomThomson opened this issue Nov 6, 2020 · 3 comments
Open

Current App Id is Updated before Previous App is Unmounted #82877

ThomThomson opened this issue Nov 6, 2020 · 3 comments
Assignees
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@ThomThomson
Copy link
Contributor

Problem
App id changes are emitted during navigateToApp - not after the previous app is unmounted. This has the potential to result in undefined behaviour.

Example
The Session service in the data plugin subscribes to appId changes in order to check that there are no active sessions during a navigation. If there is an open session, it throws a fatal error. In the new, deangularized, dashboard plugin the sessions were cleared in a useEffect cleanup function.

On an app navigation, the appId is updated before the previous app is finished unmounting, which results in a fatal error - even though the sessions are cleared immediately afterwards.

@ThomThomson ThomThomson added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Nov 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet pgayvallet self-assigned this Nov 6, 2020
@joshdover
Copy link
Contributor

Since currentAppId$ was only intended for consumption by reporting services like Telemetry, this usage is a bit unintended. I'm going to remove the bug label, but we could also consider just closing this if @ThomThomson you were able to find a workaround.

@joshdover joshdover removed the bug Fixes for quality problems that affect the customer experience label Jan 14, 2021
@ThomThomson
Copy link
Contributor Author

I was able to work around this issue by cleaning up the active session here:

We can close this, but it does seem like a natural DX to want to use currentAppId$ in this sort of way, so it might be a good idea to mention in the contract that this is not intended. @Dosant FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants