dosubot[bot] commented on issue #38926: URL: https://github.com/apache/superset/issues/38926#issuecomment-4165866248
<!-- Answer --> You're right, and I apologize for the confusion. The file was renamed from `.ts` to `.tsx` in [PR #35506](https://github.com/apache/superset/commit/8ccdf3b32b882a55f4fd1d3dbedad32541462f14), and my code index was serving a stale snapshot of the old `.ts` file from before the [major rewrite in PR #34233](https://github.com/apache/superset/commit/da55a6c94a404e3d183072d880b6ed16e4048b56). I've now confirmed against the [live `master` version](https://github.com/apache/superset/blob/master/superset-frontend/src/utils/downloadAsImage.tsx): - The file is indeed **323 lines** with `styleCache = new WeakMap<Element, CSSStyleDeclaration>()` at **line 85** - `createEnhancedClone` exists with `styleCache.delete?.(originalElement)` in its cleanup callback - Everything matches your description Your analysis looks correct to me. The `styleCache` WeakMap is module-scoped specifically so it can persist across downloads, but `styleCache.delete?.()` in the cleanup callback defeats that purpose by evicting the entry after every single download. Since the element is still mounted in the DOM, the WeakMap wouldn't GC the entry anyway — the `.delete()` call is the only thing clearing it. Removing that line should let subsequent downloads skip the expensive `getComputedStyle` traversal and reuse the cached styles, which is exactly what the cache was designed for. Would you like to open a PR for this fix? <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=brand-link).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=great_response&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=irrelevant_answer&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=incorrect_sources&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=too_verbose&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/7a222097-3ca1 -4ff2-8e3d-f301c3097bd2?feedback_type=hallucination&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=bug_report&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-bug_report) | [Other](https://app.dosu.dev/response-feedback/7a222097-3ca1-4ff2-8e3d-f301c3097bd2?feedback_type=other&utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-other)</sup> [](https://app.dosu.dev/a28d3c7e-a9d3-459e-9fb6-3a6f9ff4f357/ask?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=chat-badge) [](https://cursor.com/link/prompt?text=You%27re%20right%2C%20and%20I%20apologize%20for%20the%20confusion.%20The%20file%20was%20renamed%20from%20%60.ts%60%20to%20%60.tsx%60%20in%20%5BPR%20%2335506%5D%28https%3A//github.com/apache/superset/commit/8ccdf3b32b882a55f4fd1d3dbedad32541462f14%29%2C%20and%20my%20code%20index%20was%20serving%20a%20stale%20snapshot%20of%20the%20old%20%60.ts%60%20file%20from%20before%20the%20%5Bmajor%20rewrite%20in%20PR%20%2334233%5D%28https%3A//github.com/apache/superset/commit/da55a6c94a404e3d183072d880b6ed16e4048b56%29.%20I%27ve%20now%20confirmed%20against%20the%20%5Blive%20%60master%60%20version%5D%28https%3A//github.com/apache/superset/blob/master/superset-frontend/src/utils/downloadAs Image.tsx%29%3A%0A%0A-%20The%20file%20is%20indeed%20%2A%2A323%20lines%2A%2A%20with%20%60styleCache%20%3D%20new%20WeakMap%3CElement%2C%20CSSStyleDeclaration%3E%28%29%60%20at%20%2A%2Aline%2085%2A%2A%0A-%20%60createEnhancedClone%60%20exists%20with%20%60styleCache.delete%3F.%28originalElement%29%60%20in%20its%20cleanup%20callback%0A-%20Everything%20matches%20your%20description%0A%0AYour%20analysis%20looks%20correct%20to%20me.%20The%20%60styleCache%60%20WeakMap%20is%20module-scoped%20specifically%20so%20it%20can%20persist%20across%20downloads%2C%20but%20%60styleCache.delete%3F.%28%29%60%20in%20the%20cleanup%20callback%20defeats%20that%20purpose%20by%20evicting%20the%20entry%20after%20every%20single%20download.%20Since%20the%20element%20is%20still%20mounted%20in%20the%20DOM%2C%20the%20WeakMap%20wouldn%27t%20GC%20the%20entry%20anyway%20%E2%80%94%20the%20%60.delete%28%29%60%20call%20is%20the%20only%20thing%20clearing%20it.%20Removing%20that%20line%20should%20let%20subsequent%20downloads%20s kip%20the%20expensive%20%60getComputedStyle%60%20traversal%20and%20reuse%20the%20cached%20styles%2C%20which%20is%20exactly%20what%20the%20cache%20was%20designed%20for.%0A%0AWould%20you%20like%20to%20open%20a%20PR%20for%20this%20fix%3F) [](https://go.dosu.dev/discord-bot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=join-discord) [](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/apache/superset/issues/38926) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
