Parse feature flags into relevant config options

[alco]

Can anyone name one good reason for threading feature flags as a list of strings through internal module function calls?

They are used as a convenient way to provide tweaks from the outside. Internally, they should be parsed into relevant options at the edge.

Unfortunately, Electric has multiple input edges, so the resulting code is not as simple as I'd have liked it to be.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.18%. Comparing base (96cacdc) to head (fe3a8c3).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3517      +/-   ##
==========================================
- Coverage   75.21%   75.18%   -0.04%     
==========================================
  Files          51       51              
  Lines        2744     2744              
  Branches      409      408       -1     
==========================================
- Hits         2064     2063       -1     
- Misses        678      679       +1     
  Partials        2        2              

Flag	Coverage Δ
electric-telemetry	22.71% <ø> (ø)
elixir	57.38% <ø> (ø)
elixir-client	73.94% <ø> (ø)
packages/experimental	87.73% <ø> (ø)
packages/react-hooks	86.48% <ø> (ø)
packages/typescript-client	92.99% <ø> (-0.09%)	⬇️
packages/y-electric	55.12% <ø> (ø)
typescript	87.40% <ø> (-0.07%)	⬇️
unit-tests	75.18% <ø> (-0.04%)	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[msfstef]

Can anyone name one good reason for threading feature flags as a list of strings through internal module function calls?

I think the idea is that with a single property which is meant to hold temporary flags/guards for new features we can flexibly try out features without flooding our configs with flags that we plan to get rid of soon.

We should be validating the feature flags at the edge, but from there on I think it makes sense to keep it as a single thing (struct, list, string, whatever) to do additional guard checks for features as we try them. This way we don't add and remove flags in various parts of the system that we might end up forgetting to clean up etc.

[magnetised]

@alco though I get your point, and your first line made me laugh, actually this is a good idea. Feature flags are temporary things, we're gating wip features per-deployment, so adding and removing them should be easy. If we have to wire the config through the stack for every temporary feature flag it will be a real burden.

[alco]

I get the sentiment that we want to avoid adding config boilerplate for feature flags which are presumably temporary. But I disagree with the premise that passing flags around the codebase as a list of strings is good design.

The right mental model for feature flag is Application env: it's the globally available key-value store that any module can do lookups from. In our multitenant-capable app, Electric.StackConfig serves the equivalent role. So feature flags defined by the user should be parsed into named and typed config valuesl that are put into Electric.StackConfig and then looked up in those parts of the code that need them.

There should be no passing of feature_flags into any internal modules. Even though that's the easiest thing to do it also makes it easy to shoot oneself in the foot. Haven't we recently encountered a problem where the suspend_customers feature flags wasn't being applied after tenant restart in Cloud? :wink-wink: