codyml opened a new pull request, #21201: URL: https://github.com/apache/superset/pull/21201
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY <!--- Describe the change below, including rationale and design decisions --> This PR hopes to help future developers more easily identify issues arising from feature flag race conditions. Correct feature flag setting and reading on the frontend currently relies on JS module loading order and fails silently: - Status of feature flags is made available to front-end code via an object written to `window.featureFlags`. - This object is written when `preamble.js` (or other another module in some cases) is executed. - `window.featureFlags` is read by other modules at various points in the JS lifecycle, including module execution, React component mount, and React component update. In at least one case, a bug has been traced back to FFs being read before this object has been written. - If `window.featureFlags` does not exist, the `isFeatureEnabled` function just returns `false`. This PR keeps current behavior but prints a console error with a stack trace so developers can more easily link bugs with FF detection issues and update their code to check feature flags in a way that avoids this race condition. **An ideal place to check for feature flag status is on React component mount, but making this change to all affected code is a larger process and will be tackled in a separate PR.** ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> <img width="1075" alt="Screen Shot 2022-08-25 at 2 17 30 PM" src="https://user-images.githubusercontent.com/13007381/186770591-5ab086a9-651f-411d-b778-afbef2ec9388.png";> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> In local dev environment, you can simulate feature flag detection before initialization by replacing `superset-frontend/src/preamble.ts:67` with `setTimeout(() => initFeatureFlags(bootstrapData?.common?.feature_flags));`. Doing so should raise the console errors shown above. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] 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 -- 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]
![https://user-images.githubusercontent.com/13007381/186770591-5ab086a9-651f-411d-b778-afbef2ec9388.png"](https://user-images.githubusercontent.com/13007381/186770591-5ab086a9-651f-411d-b778-afbef2ec9388.png"){kind=link}