mikebridge opened a new pull request, #42535:
URL: https://github.com/apache/superset/pull/42535

   ### SUMMARY
   
   Selecting many metrics while creating a Semantic View crashes the 
application once the selection count passes a threshold 
([sc-107832](https://app.shortcut.com/preset/story/107832)). Semantic layers 
commonly expose large metric catalogs, so this blocks a mainstream 
configuration path.
   
   The metrics/dimensions pickers in the Add Semantic View modal are produced 
by a custom JSON Forms renderer (`MultiEnumControl` in `jsonFormsHelpers.tsx`). 
Four compounding defects combined into the crash:
   
   1. **Unbounded tag rendering** — the renderer used a *direct antd* `Select` 
with no `maxTagCount`: one tag DOM node per selected metric, all re-rendered on 
every subsequent selection. (The sibling "views" picker in the same modal uses 
Superset's wrapped Select, whose default caps tags at 4.)
   2. **Per-selection schema remount + validator-cache growth** — when the 
metrics field is a dependency of a dynamic field, each selection re-fetches the 
runtime schema; the response always got new object identities, so JSON Forms 
rebuilt its AJV validator and remounted the whole renderer tree per selection, 
while AJV cached another compiled copy of the full enum schema each time 
(cumulative memory growth).
   3. **O(N·M) re-render churn** — the JSON Forms `config` prop was an inline 
literal (new identity per render → every control re-renders per selection) and 
the renderers rebuilt (one even sorted) their full option arrays per render, 
unmemoized.
   4. **Latent infinite-update loop** — `ConstControl`/`ReadOnlyControl` 
compared array/object defaults by reference; a `default: []` field calls 
`handleChange` forever ("Maximum update depth exceeded").
   
   **The fix**
   
   - `MultiEnumControl`, `DynamicFieldControl`, and `EnumNamesControl` migrate 
to the wrapped `@superset-ui/core/components` `Select` — capped tag display 
with "+ N" overflow, `allowSelectAll={false}` (this is a pure stability fix; no 
new bulk-selection affordance), `ariaLabel`, the wrapped `optionFilterProps` 
API — with option arrays memoized. `Spin`/`Form` migrate alongside, leaving the 
file with **zero direct antd imports** (repo guideline: use wrapped component 
packages).
   - `applyRuntimeSchema` skips refresh responses whose sanitized schema is 
unchanged, so identical refreshes cause no remount and no new AJV compilation; 
the schema-refresh failure path now surfaces an error toast (previously silent) 
with all form state preserved.
   - The JSON Forms `config` prop is memoized; the views options list no longer 
sorts React state in place during render.
   - The edit modal's read-only structure tables paginate past 100 rows instead 
of rendering unboundedly.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A — no semantic-layer provider extension is available in the OSS repo to 
drive the modal end-to-end (see TESTING INSTRUCTIONS). The user-visible change: 
the metrics picker shows a capped tag summary ("+ N" overflow) instead of one 
tag per selection, matching the views picker beside it; selection state and 
persistence are unchanged.
   
   ### TESTING INSTRUCTIONS
   
   Unit tests use a provider-independent 500-value enum fixture 
(`testFixtures.ts`) — the crash mechanics are fully client-side, so no provider 
extension is required:
   
   ```
   cd superset-frontend
   npm run test -- features/semanticLayers/ features/semanticViews/
   ```
   
   Covers: tag cap + "+ N" overflow at 500 selections with the full value array 
propagated, no Select-all affordance, duplicated display labels individually 
selectable, selections surviving identical schema refreshes, `const: []` 
no-loop, failed-refresh toast with selections preserved, the persistence 
payload carrying every selected metric, and 500-row structure tables rendering 
paginated in the edit modal (small structures stay unpaginated).
   
   Manual verification in a deployment with a semantic-layer extension 
installed: create a Semantic View from a layer exposing hundreds of metrics and 
select them all — the picker stays responsive and the app does not crash; 
saving persists the full selection.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to