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: MST warning on closing map component #1269

Merged
merged 1 commit into from
May 19, 2024
Merged

Conversation

kswenson
Copy link
Member

[PT-187628522]

  1. The warning indicates that the error occurs in the selection method at DataConfigurationModel line 222.
  2. Adding if (!isAlive(self)) debugger at that line yields the stack trace in the enclosed screen shot.
  3. Clicking on the runReaction_ line in the debugger results in the following scope section of the debugger, as seen in the next screen shot.
  4. From this, we can see that the reaction in question is named observerMapPointLayer.
  5. This string doesn't appear in the code because that's the name given to observer components, so the reaction in question is the MapPointLayer component which is an observer.
  6. The fix is to test isAlive before dereferencing the dataConfiguration because there isn't a good way to tell an observer component to stop observing.
MSTWarning MSTWarningScope

@kswenson kswenson requested review from bfinzer and emcelroy May 18, 2024 01:16
Copy link

codecov bot commented May 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.20%. Comparing base (8087dc0) to head (3268d57).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1269      +/-   ##
==========================================
+ Coverage   85.08%   85.20%   +0.12%     
==========================================
  Files         482      482              
  Lines       22959    22960       +1     
  Branches     6116     5736     -380     
==========================================
+ Hits        19534    19564      +30     
+ Misses       3260     3231      -29     
  Partials      165      165              
Flag Coverage Δ
cypress 73.83% <ø> (+0.15%) ⬆️
jest 52.15% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@bfinzer bfinzer left a comment

Choose a reason for hiding this comment

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

👍🏻LGTM
I find this to be a subtle bug! Thanks for tracking it down.

@kswenson kswenson merged commit d453582 into main May 19, 2024
8 checks passed
@kswenson kswenson deleted the 187628522-fix-map-warning branch May 19, 2024 12:45
@kswenson kswenson added the v3 CODAP v3 label May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 CODAP v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants