fix(gatsby): honor flags disabled via config when deciding wether to …

…add included flags (#30992)
gatsbyjs · Apr 22, 2021 · e7327c3 · e7327c3
1 parent 24212b8
commit e7327c3
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 24 deletions.
diff --git a/packages/gatsby/src/utils/__tests__/__snapshots__/handle-flags.ts.snap b/packages/gatsby/src/utils/__tests__/__snapshots__/handle-flags.ts.snap
@@ -50,9 +50,8 @@ The following flags were automatically enabled on your site:
 - PARTIAL_RELEASE · (Umbrella Issue (test)) · test
 - PARTIAL_RELEASE_ONLY_NEW_LODASH · (Umbrella Issue (test)) · test
 
-There are 3 other flags available that you might be interested in:
+There are 2 other flags available that you might be interested in:
 - FAST_DEV · Enable all experiments aimed at improving develop server start time
-- DEV_SSR · (Umbrella Issue (https://github.com/gatsbyjs/gatsby/discussions/28138)) · SSR pages on full reloads during develop. Helps you detect SSR bugs and fix them without needing to do full builds.
 - YET_ANOTHER · (Umbrella Issue (test)) · test
 ",
   "unknownFlagMessage": "",
@@ -95,6 +94,12 @@ flags: {
 The following flags were automatically enabled on your site:
 - PARTIAL_RELEASE · (Umbrella Issue (test)) · test
 - PARTIAL_RELEASE_ONLY_NEW_LODASH · (Umbrella Issue (test)) · test
+
+There are 4 other flags available that you might be interested in:
+- FAST_DEV · Enable all experiments aimed at improving develop server start time
+- DEV_SSR · (Umbrella Issue (https://github.com/gatsbyjs/gatsby/discussions/28138)) · SSR pages on full reloads during develop. Helps you detect SSR bugs and fix them without needing to do full builds.
+- ALL_COMMANDS · (Umbrella Issue (test)) · test
+- YET_ANOTHER · (Umbrella Issue (test)) · test
 ",
   "unknownFlagMessage": "",
 }
@@ -247,6 +252,9 @@ flags: {
 The following flags were automatically enabled on your site:
 - PARTIAL_RELEASE · (Umbrella Issue (test)) · test
 - PARTIAL_RELEASE_ONLY_NEW_LODASH · (Umbrella Issue (test)) · test
+
+There is one other flag available that you might be interested in:
+- YET_ANOTHER · (Umbrella Issue (test)) · test
 ",
   "unknownFlagMessage": "",
 }

diff --git a/packages/gatsby/src/utils/__tests__/handle-flags.ts b/packages/gatsby/src/utils/__tests__/handle-flags.ts
@@ -261,12 +261,10 @@ describe(`handle flags`, () => {
       - ALWAYS_OPT_IN · (Umbrella Issue (test)) · test
       - DEV_SSR · (Umbrella Issue (https://github.com/gatsbyjs/gatsby/discussions/28138)) · SSR pages on full reloads during develop. Helps you detect SSR bugs and fix them without needing to do full builds.
 
-      There are 5 other flags available that you might be interested in:
+      There are 3 other flags available that you might be interested in:
       - FAST_DEV · Enable all experiments aimed at improving develop server start time
       - ALL_COMMANDS · (Umbrella Issue (test)) · test
       - YET_ANOTHER · (Umbrella Issue (test)) · test
-      - PARTIAL_RELEASE · (Umbrella Issue (test)) · test
-      - PARTIAL_RELEASE_ONLY_NEW_LODASH · (Umbrella Issue (test)) · test
       "
     `)
   })
@@ -441,8 +439,120 @@ describe(`handle flags`, () => {
         - FAST_DEV · Enable all experiments aimed at improving develop server start time
         - ALL_COMMANDS · (Umbrella Issue (test)) · test
         - YET_ANOTHER · (Umbrella Issue (test)) · test
-        - PARTIAL_RELEASE · (Umbrella Issue (test)) · test
-        - PARTIAL_RELEASE_ONLY_NEW_LODASH · (Umbrella Issue (test)) · test
+        "
+      `)
+    })
+  })
+
+  describe(`includeFlags`, () => {
+    it(`enabling umbrella flag implies enabling individual flags`, () => {
+      const response = handleFlags(
+        [
+          {
+            name: `UMBRELLA`,
+            env: `GATSBY_UMBRELLA`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+            includedFlags: [`SUB_FLAG_A`, `SUB_FLAG_B`],
+          },
+          {
+            name: `SUB_FLAG_A`,
+            env: `GATSBY_SUB_FLAG_A`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+          },
+          {
+            name: `SUB_FLAG_B`,
+            env: `GATSBY_SUB_FLAG_B`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+          },
+        ],
+        {
+          UMBRELLA: true,
+        },
+        `build`
+      )
+
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `SUB_FLAG_A` })
+      )
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `SUB_FLAG_B` })
+      )
+      expect(response.message).toMatchInlineSnapshot(`
+        "The following flags are active:
+        - UMBRELLA · (Umbrella Issue (test)) · test
+        - SUB_FLAG_A · (Umbrella Issue (test)) · test
+        - SUB_FLAG_B · (Umbrella Issue (test)) · test
+        "
+      `)
+    })
+
+    it(`allow disabling individual flags that umbrella flag would enable`, () => {
+      const response = handleFlags(
+        [
+          {
+            name: `UMBRELLA`,
+            env: `GATSBY_UMBRELLA`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+            includedFlags: [`SUB_FLAG_A`, `SUB_FLAG_B`],
+          },
+          {
+            name: `SUB_FLAG_A`,
+            env: `GATSBY_SUB_FLAG_A`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+          },
+          {
+            name: `SUB_FLAG_B`,
+            env: `GATSBY_SUB_FLAG_B`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            testFitness: (): fitnessEnum => true,
+          },
+        ],
+        {
+          UMBRELLA: true,
+          SUB_FLAG_B: false,
+        },
+        `build`
+      )
+
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `SUB_FLAG_A` })
+      )
+      expect(response.enabledConfigFlags).not.toContainEqual(
+        expect.objectContaining({ name: `SUB_FLAG_B` })
+      )
+      expect(response.message).toMatchInlineSnapshot(`
+        "The following flags are active:
+        - UMBRELLA · (Umbrella Issue (test)) · test
+        - SUB_FLAG_A · (Umbrella Issue (test)) · test
         "
       `)
     })

diff --git a/packages/gatsby/src/utils/handle-flags.ts b/packages/gatsby/src/utils/handle-flags.ts
@@ -122,8 +122,14 @@ const handleFlags = (
       flag.includedFlags.forEach(includedName => {
         const incExp = flags.find(e => e.name == includedName)
         if (incExp) {
-          enabledConfigFlags.push(incExp)
-          addIncluded(incExp)
+          const flagIsDisabledByUser =
+            typeof configFlags[includedName] !== `undefined` &&
+            !configFlags[includedName]
+
+          if (!flagIsDisabledByUser) {
+            enabledConfigFlags.push(incExp)
+            addIncluded(incExp)
+          }
         }
       })
     }
@@ -197,23 +203,28 @@ The following flags were automatically enabled on your site:`
       })
     }
 
-    const otherFlagsCount = applicableFlags.size - enabledConfigFlags.length
-    // Check if there is other flags and if the user actually set any flags themselves.
-    // Don't count flags they were automatically opted into.
-    if (otherFlagsCount > 0 && Object.keys(configFlags).length > 0) {
+    const otherFlagSuggestionLines: Array<string> = []
+    const enabledFlagsSet = new Set()
+    enabledConfigFlags.forEach(f => enabledFlagsSet.add(f.name))
+    applicableFlags.forEach(flag => {
+      if (
+        !enabledFlagsSet.has(flag.name) &&
+        typeof configFlags[flag.name] === `undefined`
+      ) {
+        // we want to suggest flag when it's not enabled and user specifically didn't use it in config
+        // we don't want to suggest flag user specifically wanted to disable
+        otherFlagSuggestionLines.push(generateFlagLine(flag))
+      }
+    })
+
+    if (otherFlagSuggestionLines.length > 0) {
       message += `\n\nThere ${
-        otherFlagsCount === 1
+        otherFlagSuggestionLines.length === 1
           ? `is one other flag`
-          : `are ${otherFlagsCount} other flags`
-      } available that you might be interested in:`
-
-      const enabledFlagsSet = new Set()
-      enabledConfigFlags.forEach(f => enabledFlagsSet.add(f.name))
-      applicableFlags.forEach(flag => {
-        if (!enabledFlagsSet.has(flag.name)) {
-          message += generateFlagLine(flag)
-        }
-      })
+          : `are ${otherFlagSuggestionLines.length} other flags`
+      } available that you might be interested in:${otherFlagSuggestionLines.join(
+        ``
+      )}`
     }
 
     if (message.length > 0) {