fix(image): unregisterPreviewUrl never removes an entry from the map - #8171
Open
MILLERMARRU wants to merge 1 commit into
Open
fix(image): unregisterPreviewUrl never removes an entry from the map#8171MILLERMARRU wants to merge 1 commit into
MILLERMARRU wants to merge 1 commit into
Conversation
Map.prototype.delete() is a no-op on a missing key, so guarding the delete call with `!has(sid)` only fires in exactly the case where it does nothing, and skips it whenever the key is actually present. An n-image inside an n-image-group never actually gets removed from registeredImageUrlMap on unmount. Fixes tusen-ai#8170
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
n-image-group'sunregisterPreviewUrlhad an inverted guard, so it never actually removed an entry fromregisteredImageUrlMapon unmount.Map.prototype.delete()is a no-op when the key is already absent, so this only callsdelete()in exactly the case where it does nothing (key already gone), and skips it in the normal unmount case where the key is actually present. So anyn-imagerendered inside ann-image-groupleaves its URL registered inregisteredImageUrlMapforever, even after the component unmounts, growing the map with stale entries for images that no longer exist in the DOM.Fixes #8170
Fix
One-line inversion of the condition:
Scope
This is separate from #8158/#8168 (the
registerImageUrlre-registration guard using the wrong key), which is already being fixed in that other PR. This one only touches the unregister path in the same function.I didn't add a new unit test since
registeredImageUrlMapisn't exposed on the component's public instance for a black-box assertion, andimageIdis a module-leveluuid++counter (never reused across mounts), so there's no externally-observable symptom to assert on without reaching into internals. Happy to add one if there's a preferred pattern for asserting on this kind of internal state in this codebase.