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

fix: Expose TableSubscription.close() to JS (#6446) #6448

Open
wants to merge 2 commits into
base: rc/v0.37.x
Choose a base branch
from

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Nov 29, 2024

  • The close() method is only on AbstractTableSubscription, and it appears that it doesn't automatically get added to the JS API even though TableSubscription is defined as a JsType
  • Explicitly add the method to TableSubscription as well.
  • Fixes Can't switch between tabs that contain dx plots #6447

- The close() method is only on `AbstractTableSubscription`, and it
appears that it doesn't automatically get added to the JS API even
though `TableSubscription` is defined as a `JsType`
- Explicitly add the method to `TableSubscription` as well.
- Fixes deephaven#6447
@mofojed mofojed self-assigned this Nov 29, 2024
@mofojed mofojed added NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Nov 29, 2024
nbauernfeind
nbauernfeind previously approved these changes Nov 29, 2024
@mofojed mofojed added this to the 0.37.1 milestone Nov 29, 2024
Copy link
Member

@niloc132 niloc132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review #6452 so we can land that to main and cherrypick those changed instead - this way we don't need to redefine the method just to export it.

Also please avoid implementation details in javadoc, which is exported in the js/ts docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants