fix(common): manual impl Default for FeatureFlags to align FeatureFlags::default() with Serde per-field default

[ansemb]

Fixes #5063

Problem

enforce_fragment_alias_where_ambiguous was Enabled only when deserialized via Serde (using #[serde(default = "enabled_feature_flag")]), but FeatureFlags::default() produced Disabled because #[derive(Default)] ignores Serde attributes.

This was the reason that adding an empty object to the featureFlags enabled the enforce_fragment_alias_where_ambiguous, but omitting it didn't:

{
  //...
  "projects": {
    "test": {
      //...
      "featureFlags": {}
    }
  }
}

Changes

• Implemented a manual Default for FeatureFlags that sets enforce_fragment_alias_where_ambiguous to FeatureFlag::Enabled and leaves all other fields at their existing defaults.
• Derived PartialEq for FeatureFlags, FeatureFlag, Rollout and RolloutRange to enable object equality in tests.

NOTE: This is probably a breaking change, since we're now (actually) enabling the flag by default.

Tests

Updated/added unit tests in feature_flags.rs:

• default_trait_sets_enforce_fragment_alias_enabled ensures the Rust default enables the field (could be omitted?).
• serde_default_from_empty_object_matches_intent asserts deserializing {} equals FeatureFlags::default().

[meta-cla]

Hi @ansemb!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[ansemb]

Running this test is failing with @alias error:

✖ Expected `@alias` directive. `spreadOfAssignableFragmentAssignable_user` is defined on `User` which might not match this selection type of `Node`. Add `@alias` to this spread to expose the fragment reference as a nullable property.

  spread-of-assignable-fragment.graphql:9:8
    8 │   node2: node(id: 5) {
    9 │     ...spreadOfAssignableFragmentAssignable_user
      │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   10 │     ...spreadOfAssignableFragmentAssignable_node

However, when adding an @alias we get this error:

✖ Because an assignable fragment was spread in this linked field, each of this linked field's selections must be an inline fragment with no directives, refining the type to a unique concrete type and containing an unaliased __typename field with no directives. However, an inline fragment in this linked field contains directives.

  spread-of-assignable-fragment.graphql:8:10
    7 │   }
    8 │   node2: node(id: 5) {
      │          ^^^^
    9 │     ...spreadOfAssignableFragmentAssignable_user @alias


✖ Because an assignable fragment was spread in this linked field, each of this linked field's selections must be an inline fragment with no directives, refining the type to a unique concrete type and containing an unaliased __typename field with no directives. However, an inline fragment in this linked field does not contain an unaliased __typename selection with no directives.

  spread-of-assignable-fragment.graphql:8:10
    7 │   }
    8 │   node2: node(id: 5) {
      │          ^^^^
    9 │     ...spreadOfAssignableFragmentAssignable_user @alias


✖ Because an assignable fragment was spread in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include.

  spread-of-assignable-fragment.graphql:10:8
    9 │     ...spreadOfAssignableFragmentAssignable_user @alias
   10 │     ...spreadOfAssignableFragmentAssignable_node
      │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   11 │   }

  ℹ enclosing linked field

  spread-of-assignable-fragment.graphql:8:10
    7 │   }
    8 │   node2: node(id: 5) {
      │          ^^^^
    9 │     ...spreadOfAssignableFragmentAssignable_user @alias


✖ The @alias directive is not allowed on assignable fragment spreads.

  spread-of-assignable-fragment.graphql:9:50
    8 │   node2: node(id: 5) {
    9 │     ...spreadOfAssignableFragmentAssignable_user @alias
      │                                                  ^^^^^^
   10 │     ...spreadOfAssignableFragmentAssignable_node

For now I've removed this spread, but I'm ot sure what the prefered solution is.

[facebook-github-bot]

@evanyeung has imported this pull request. If you are a Meta employee, you can view this in D81642130.

[facebook-github-bot]

@evanyeung merged this pull request in be2c2a0.