villebro commented on PR #38851: URL: https://github.com/apache/superset/pull/38851#issuecomment-4164564896
> > While I don't love deep prop drilling, It appears this is indeed the best solution to guarantee no race condition. However, to guarantee we don't run into this race condition elsewhere, I think we should make the explicit search term required, and not fall back to `window.location.search`. WDYT? > > The upstream refactor in Chart/index.tsx no longer reads URL params at initialization at all — it passes loc explicitly through loadExploreData(loc, saveAction), so any getUrlParam call inside that function is automatically safe. > > For the remaining ~25 call sites, the race condition genuinely can't happen there — they're all reading URL params either at user interaction time or in stable post-load renders, not in an async initialization gap. So making search required would impose real boilerplate cost (threading location.search through hooks, class components, thunks, and utility functions) across call sites that have no actual exposure to the bug. Makes sense, thanks for the thorough investigation Madhu! -- 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]
