dosubot[bot] commented on issue #39006: URL: https://github.com/apache/superset/issues/39006#issuecomment-4169768971
<!-- Greeting --> Hi @iskanderknani2005-oss! I'm [Dosu](https://go.dosu.dev/dosubot) and I’m helping the superset team. <!-- Answer --> Looking at the [actual implementation](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts#L26-L45), the behavior is intentional — the docstring explicitly states: *"Any circular references are simply omitted, as if there had been no data present"* [[1]](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts#L20-L24). So the silent omission is by design, not a bug. That said, your point about it being a poor default is fair. A `"[Circular]"` placeholder would be more debuggable than silently dropping keys. A few notes on the current logic: - **Lines 30–38**: When a repeated object is detected, the code tries `JSON.parse(JSON.stringify(value))` to distinguish a legitimate repeated reference from a true circular reference [[2]](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts#L32-L38). Only if that inner stringify throws (i.e., it's genuinely circular) does it return `undefined` to omit the key. - **The reproduction in the issue** actually demonstrates the *fixed* replacer (your proposed version), not the current behavior. The current code would produce `{"name":"test"}` for your example — `data` is silently dropped, not the whole object becoming `{}`. - **Memory concern**: The `Set` is scoped to a single `safeStringify` call and gets GC'd after the function returns [[3]](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts#L27). It doesn't persist across calls, so there's no leak — though it does hold references for the duration of serialization of a single large object. This function is used in ~9 places across the frontend, including chart form data, dashboard state, and logging [[4]](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts). There's also a duplicate implementation in the [DeckGL plugin](https://github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utils/safeStringify.ts). If you'd like to contribute a fix, replacing `return;` on line 37 with `return '[Circular]';` would be a minimal, backward-compatible improvement that preserves the key in the output with a clear marker. A `console.warn` could also be added there for debuggability. <!-- 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?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-great_response#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=great_response) | [Irrelevant](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-irrelevant_answer#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-incorrect_sources#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-too_verbose#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=too_verbose) | [Hallucination](https:// app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-hallucination#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-bug_report#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=bug_report) | [Other](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-other#message_id=801be034-3b9d-40ac-a0e4-4a3ac5036778&feedback_type=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=Looking%20at%20the%20%5Bactual%20implementation%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts%23L26-L45%29%2C%20the%20behavior%20is%20intentional%20%E2%80%94%20the%20docstring%20explicitly%20states%3A%20%2A%22Any%20circular%20references%20are%20simply%20omitted%2C%20as%20if%20there%20had%20been%20no%20data%20present%22%2A%20%5B%5B1%5D%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts%23L20-L24%29.%20So%20the%20silent%20omission%20is%20by%20design%2C%20not%20a%20bug.%0A%0AThat%20said%2C%20your%20point%20abou t%20it%20being%20a%20poor%20default%20is%20fair.%20A%20%60%22%5BCircular%5D%22%60%20placeholder%20would%20be%20more%20debuggable%20than%20silently%20dropping%20keys.%20A%20few%20notes%20on%20the%20current%20logic%3A%0A%0A-%20%2A%2ALines%2030%E2%80%9338%2A%2A%3A%20When%20a%20repeated%20object%20is%20detected%2C%20the%20code%20tries%20%60JSON.parse%28JSON.stringify%28value%29%29%60%20to%20distinguish%20a%20legitimate%20repeated%20reference%20from%20a%20true%20circular%20reference%20%5B%5B2%5D%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts%23L32-L38%29.%20Only%20if%20that%20inner%20stringify%20throws%20%28i.e.%2C%20it%27s%20genuinely%20circular%29%20does%20it%20return%20%60undefined%60%20to%20omit%20the%20key.%0A%0A-%20%2A%2AThe%20reproduction%20in%20the%20issue%2A%2A%20actually%20demonstrates%20the%20%2Afixed%2A%20replacer%20%28your%20proposed%20version%29%2C%20not%20the%20current%20behavior.%20The%2 0current%20code%20would%20produce%20%60%7B%22name%22%3A%22test%22%7D%60%20for%20your%20example%20%E2%80%94%20%60data%60%20is%20silently%20dropped%2C%20not%20the%20whole%20object%20becoming%20%60%7B%7D%60.%0A%0A-%20%2A%2AMemory%20concern%2A%2A%3A%20The%20%60Set%60%20is%20scoped%20to%20a%20single%20%60safeStringify%60%20call%20and%20gets%20GC%27d%20after%20the%20function%20returns%20%5B%5B3%5D%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/src/utils/safeStringify.ts%23L27%29.%20It%20doesn%27t%20persist%20across%20calls%2C%20so%20there%27s%20no%20leak%20%E2%80%94%20though%20it%20does%20hold%20references%20for%20the%20duration%20of%20serialization%20of%20a%20single%20large%20object.%0A%0AThis%20function%20is%20used%20in%20~9%20places%20across%20the%20frontend%2C%20including%20chart%20form%20data%2C%20dashboard%20state%2C%20and%20logging%20%5B%5B4%5D%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45 377799c/superset-frontend/src/utils/safeStringify.ts%29.%20There%27s%20also%20a%20duplicate%20implementation%20in%20the%20%5BDeckGL%20plugin%5D%28https%3A//github.com/apache/superset/blob/829e4d92d91ceae4b43b1ed3b063ffe45377799c/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utils/safeStringify.ts%29.%0A%0AIf%20you%27d%20like%20to%20contribute%20a%20fix%2C%20replacing%20%60return%3B%60%20on%20line%2037%20with%20%60return%20%27%5BCircular%5D%27%3B%60%20would%20be%20a%20minimal%2C%20backward-compatible%20improvement%20that%20preserves%20the%20key%20in%20the%20output%20with%20a%20clear%20marker.%20A%20%60console.warn%60%20could%20also%20be%20added%20there%20for%20debuggability.) [](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/39006) -- 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]
