fix: Add warnings if feature flags are read before initialization

[codyml]

SUMMARY

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

TESTING INSTRUCTIONS

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

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • 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

[github-actions]

Storybook has completed and can be viewed at

[samtfm]

excellent improvement!

[codecov]

Codecov Report

Merging #21201 (20298a5) into master (17ad0d8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #21201   +/-   ##
=======================================
  Coverage   66.39%   66.39%           
=======================================
  Files        1781     1781           
  Lines       67893    67901    +8     
  Branches     7248     7246    -2     
=======================================
+ Hits        45077    45085    +8     
  Misses      20954    20954           
  Partials     1862     1862           

Flag	Coverage Δ
javascript	52.31% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ackages/superset-ui-core/src/utils/featureFlags.ts	100.00% <100.00%> (ø)
superset-frontend/src/featureFlags.ts	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rusackas]

I think this is fine for this PR, but it seems like we should just be importing the above Superset UI module everywhere and avoid having duplicate code in the project. Maybe we can clean that up in some other PR.

[rusackas]

LGTM... though we can probably DRY the project up a little bit ;)

[michael-s-molina]

Unfortunately, when executing the tests, this change fills the logs with errors as an unintended effect. @codyml Can you provide a patch for this when running tests?

[codyml]

Unfortunately, when executing the tests, this change fills the logs with errors as an unintended effect. @codyml Can you provide a patch for this when running tests?

Yikes, looking into this ASAP. Sorry!