chore(gatsby): enable query on demand (and lazy images) by default for local development (#28787)

pieh · vladar · web-flow · commit f19c8072b4a8 · 2020-12-30T03:31:19.000+07:00
* feat(flags): add LOCKED_IN mode for flags for cases where opt-in features become defaults

* feat(gatsby): enable query on demand (and lazy images) by default for local development

* fix "lint"

* chore(telemetry): don't track usage of query on demand / lazy images anymore - those are defaults

* use plain false instead of weird Symbol do opt out of telemetry tracking

* Update packages/gatsby/src/utils/handle-flags.ts

Co-authored-by: Vladimir Razuvaev &lt;vladimir.razuvaev@gmail.com&gt;

* update snapshots

Co-authored-by: Vladimir Razuvaev &lt;vladimir.razuvaev@gmail.com&gt;
diff --git a/packages/gatsby/src/services/initialize.ts b/packages/gatsby/src/services/initialize.ts
@@ -28,6 +28,7 @@ import { IPluginInfoOptions } from "../bootstrap/load-plugins/types"
 import { internalActions } from "../redux/actions"
 import { IGatsbyState } from "../redux/types"
 import { IBuildContext } from "./types"
+import availableFlags from "../utils/flags"
 
 interface IPluginResolution {
   resolve: string
@@ -41,18 +42,17 @@ if (
   process.env.GATSBY_EXPERIMENTAL_FAST_DEV &&
   !isCI()
 ) {
-  process.env.GATSBY_EXPERIMENTAL_LAZY_IMAGES = `true`
-  process.env.GATSBY_EXPERIMENTAL_QUERY_ON_DEMAND = `true`
   process.env.GATSBY_EXPERIMENTAL_DEV_SSR = `true`
+  process.env.PRESERVE_FILE_DOWNLOAD_CACHE = `true`
+  process.env.PRESERVE_WEBPACK_CACHE = `true`
 
   reporter.info(`
-Three fast dev experiments are enabled: Query on Demand, Development SSR, and Lazy Images (only with gatsby-plugin-sharp@^2.10.0).
+Three fast dev experiments are enabled: Development SSR, preserving file download cache and preserving webpack cache.
 
 Please give feedback on their respective umbrella issues!
 
-- https://gatsby.dev/query-on-demand-feedback
 - https://gatsby.dev/dev-ssr-feedback
-- https://gatsby.dev/lazy-images-feedback
+- https://gatsby.dev/cache-clearing-feedback
   `)
 
   telemetry.trackFeatureIsUsed(`FastDev`)
@@ -194,7 +194,6 @@ export async function initialize({
       )
     }
 
-    const availableFlags = require(`../utils/flags`).default
     // Get flags
     const { enabledConfigFlags, unknownFlagMessage, message } = handleFlags(
       availableFlags,
@@ -217,7 +216,9 @@ export async function initialize({
 
     //  track usage of feature
     enabledConfigFlags.forEach(flag => {
-      telemetry.trackFeatureIsUsed(flag.telemetryId)
+      if (flag.telemetryId) {
+        telemetry.trackFeatureIsUsed(flag.telemetryId)
+      }
     })
 
     // Track the usage of config.flags
@@ -282,15 +283,9 @@ export async function initialize({
       delete process.env.GATSBY_EXPERIMENTAL_QUERY_ON_DEMAND
     } else if (isCI()) {
       delete process.env.GATSBY_EXPERIMENTAL_QUERY_ON_DEMAND
-      reporter.warn(
-        `Experimental Query on Demand feature is not available in CI environment. Continuing with regular mode.`
+      reporter.verbose(
+        `Experimental Query on Demand feature is not available in CI environment. Continuing with eager query running.`
       )
-    } else {
-      // We already show a notice for this flag.
-      if (!process.env.GATSBY_EXPERIMENTAL_FAST_DEV) {
-        reporter.info(`Using experimental Query on Demand feature`)
-      }
-      telemetry.trackFeatureIsUsed(`QueryOnDemand`)
     }
   }
 
diff --git a/packages/gatsby/src/utils/__tests__/__snapshots__/handle-flags.ts.snap b/packages/gatsby/src/utils/__tests__/__snapshots__/handle-flags.ts.snap
@@ -85,9 +85,7 @@ Object {
       "umbrellaIssue": "test",
     },
   ],
-  "message": "
-
-We're shipping new features! For final testing, we're rolling them out first to a small % of Gatsby users
+  "message": "We're shipping new features! For final testing, we're rolling them out first to a small % of Gatsby users
 and your site was automatically chosen as one of them. With your help, we'll then release them to everyone in the next minor release.
 
 We greatly appreciate your help testing the change. Please report any feedback good or bad in the umbrella issue. If you do encounter problems, please disable the flag by setting it to false in your gatsby-config.js like:
diff --git a/packages/gatsby/src/utils/__tests__/handle-flags.ts b/packages/gatsby/src/utils/__tests__/handle-flags.ts
@@ -276,4 +276,129 @@ describe(`handle flags`, () => {
       "
     `)
   })
+
+  describe(`LOCKED_IN`, () => {
+    it(`Enables locked in flag by default and doesn't mention it in terminal (no spam)`, () => {
+      const response = handleFlags(
+        [
+          {
+            name: `ALWAYS_LOCKED_IN`,
+            env: `GATSBY_ALWAYS_LOCKED_IN`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            // this will always LOCKED IN
+            testFitness: (): fitnessEnum => `LOCKED_IN`,
+          },
+        ],
+        {},
+        `develop`
+      )
+
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `ALWAYS_LOCKED_IN` })
+      )
+      expect(response.message).toEqual(``)
+    })
+
+    it(`Display message saying config flag for LOCKED_IN feature is no-op`, () => {
+      const response = handleFlags(
+        [
+          {
+            name: `ALWAYS_LOCKED_IN_SET_IN_CONFIG`,
+            env: `GATSBY_ALWAYS_LOCKED_IN_SET_IN_CONFIG`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            // this will always LOCKED IN
+            testFitness: (): fitnessEnum => `LOCKED_IN`,
+          },
+        ],
+        {
+          // this has no effect, but we want to show to user that
+          ALWAYS_LOCKED_IN_SET_IN_CONFIG: true,
+        },
+        `develop`
+      )
+
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `ALWAYS_LOCKED_IN_SET_IN_CONFIG` })
+      )
+      expect(response.message).toMatchInlineSnapshot(`
+        "Some features you configured with flags are used natively now.
+        Those flags no longer have any effect and you can remove them from config:
+        - ALWAYS_LOCKED_IN_SET_IN_CONFIG · (Umbrella Issue (test)) · test
+        "
+      `)
+    })
+
+    it(`Kitchen sink`, () => {
+      const response = handleFlags(
+        activeFlags.concat([
+          {
+            name: `ALWAYS_LOCKED_IN`,
+            env: `GATSBY_ALWAYS_LOCKED_IN`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            // this will always LOCKED IN
+            testFitness: (): fitnessEnum => `LOCKED_IN`,
+          },
+          {
+            name: `ALWAYS_LOCKED_IN_SET_IN_CONFIG`,
+            env: `GATSBY_ALWAYS_LOCKED_IN_SET_IN_CONFIG`,
+            command: `all`,
+            description: `test`,
+            umbrellaIssue: `test`,
+            telemetryId: `test`,
+            experimental: false,
+            // this will always LOCKED IN
+            testFitness: (): fitnessEnum => `LOCKED_IN`,
+          },
+        ]),
+        {
+          ALWAYS_OPT_IN: true,
+          DEV_SSR: true,
+          PARTIAL_RELEASE: false,
+          PARTIAL_RELEASE_ONLY_NEW_LODASH: false,
+          // this has no effect, but we want to show to user that
+          ALWAYS_LOCKED_IN_SET_IN_CONFIG: true,
+        },
+        `develop`
+      )
+
+      // this is enabled, but because it's not configurable anymore and user doesn't set it explicitly in config - there is no point in printing information about it
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `ALWAYS_LOCKED_IN` })
+      )
+      // this is enabled, but because it's not configurable anymore and user sets it in config - we want to mention that this config flag has no effect anymore
+      expect(response.enabledConfigFlags).toContainEqual(
+        expect.objectContaining({ name: `ALWAYS_LOCKED_IN_SET_IN_CONFIG` })
+      )
+      expect(response.message).toMatchInlineSnapshot(`
+        "The following flags are active:
+        - 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.
+
+        Some features you configured with flags are used natively now.
+        Those flags no longer have any effect and you can remove them from config:
+        - ALWAYS_LOCKED_IN_SET_IN_CONFIG · (Umbrella Issue (test)) · test
+
+        There are 5 other flags available that you might be interested in:
+        - FAST_DEV · Enable all experiments aimed at improving develop server start time
+        - QUERY_ON_DEMAND · (Umbrella Issue (https://github.com/gatsbyjs/gatsby/discussions/27620)) · Only run queries when needed instead of running all queries upfront. Speeds starting the develop server.
+        - ONLY_BUILDS · (Umbrella Issue (test)) · test
+        - 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
+        "
+      `)
+    })
+  })
 })
diff --git a/packages/gatsby/src/utils/flags.ts b/packages/gatsby/src/utils/flags.ts
@@ -1,4 +1,3 @@
-import sampleSiteForExperiment from "./sample-site-for-experiment"
 import _ from "lodash"
 import semver from "semver"
 
@@ -29,14 +28,17 @@ export const satisfiesSemvers = (
   return result
 }
 
-export type fitnessEnum = true | false | "OPT_IN"
+export type fitnessEnum = true | false | "OPT_IN" | "LOCKED_IN"
 
 export interface IFlag {
   name: string
   env: string
   description: string
   command: executingCommand
-  telemetryId: string
+  /**
+   * Use string identifier to track enabled flag or false to disable any tracking (useful when flag becomes new defaults)
+   */
+  telemetryId: string | false
   // Heuristics for deciding if a flag is experimental:
   // - there are known bugs most people will encounter and that block being
   // able to use Gatsby normally
@@ -47,13 +49,20 @@ export interface IFlag {
   // resolved and ~50+ people have tested it, experimental should be set to
   // false.
   experimental: boolean
-  // Generally just return true.
-  //
-  // False means it'll be disabled despite the user setting it true e.g.
-  // it just won't work e.g. it doesn't have new enough version for something.
-  //
-  // OPT_IN means the gatsby will enable the flag (unless the user explicitly
-  // disablees it.
+  /**
+   * True means conditions for the feature are met and can be opted in by user.
+   *
+   * False means it'll be disabled despite the user setting it true e.g.
+   * it just won't work e.g. it doesn't have new enough version for something.
+   *
+   * OPT_IN means the gatsby will enable the flag (unless the user explicitly
+   * disables it.
+   *
+   * LOCKED_IN means that feature is enabled always (unless `noCI` condition is met).
+   * This is mostly to provide more meaningful terminal messages instead of removing
+   * flag from the flag list when users has the flag set in configuration
+   * (avoids showing unknown flag message and shows "no longer needed" message).
+   */
   testFitness: (flag: IFlag) => fitnessEnum
   includedFlags?: Array<string>
   umbrellaIssue?: string
@@ -70,8 +79,6 @@ const activeFlags: Array<IFlag> = [
     description: `Enable all experiments aimed at improving develop server start time`,
     includedFlags: [
       `DEV_SSR`,
-      `QUERY_ON_DEMAND`,
-      `LAZY_IMAGES`,
       `PRESERVE_FILE_DOWNLOAD_CACHE`,
       `PRESERVE_WEBPACK_CACHE`,
     ],
@@ -91,70 +98,33 @@ const activeFlags: Array<IFlag> = [
     name: `QUERY_ON_DEMAND`,
     env: `GATSBY_EXPERIMENTAL_QUERY_ON_DEMAND`,
     command: `develop`,
-    telemetryId: `QueryOnDemand`,
+    telemetryId: false,
     experimental: false,
     description: `Only run queries when needed instead of running all queries upfront. Speeds starting the develop server.`,
     umbrellaIssue: `https://gatsby.dev/query-on-demand-feedback`,
     noCI: true,
-    testFitness: (): fitnessEnum => {
-      // Take a 10% of slice of users.
-      if (sampleSiteForExperiment(`QUERY_ON_DEMAND`, 10)) {
-        let isPluginSharpNewEnoughOrNotInstalled = false
-        try {
-          // Try requiring plugin-sharp so we know if it's installed or not.
-          // If it does, we also check if it's new enough.
-          // eslint-disable-next-line
-          require.resolve(`gatsby-plugin-sharp`)
-          const semverConstraints = {
-            // Because of this, this flag will never show up
-            "gatsby-plugin-sharp": `>=2.10.0`,
-          }
-          if (satisfiesSemvers(semverConstraints)) {
-            isPluginSharpNewEnoughOrNotInstalled = true
-          }
-        } catch (e) {
-          if (e.code === `MODULE_NOT_FOUND`) {
-            isPluginSharpNewEnoughOrNotInstalled = true
-          }
-        }
-
-        if (isPluginSharpNewEnoughOrNotInstalled) {
-          return `OPT_IN`
-        } else {
-          // Don't opt them in but they can still manually enable.
-          return true
-        }
-      } else {
-        // Don't opt them in but they can still manually enable.
-        return true
-      }
-    },
+    testFitness: (): fitnessEnum => `LOCKED_IN`,
   },
   {
     name: `LAZY_IMAGES`,
     env: `GATSBY_EXPERIMENTAL_LAZY_IMAGES`,
     command: `develop`,
-    telemetryId: `LazyImageProcessing`,
+    telemetryId: false,
     experimental: false,
     description: `Don't process images during development until they're requested from the browser. Speeds starting the develop server. Requires gatsby-plugin-sharp@2.10.0 or above.`,
     umbrellaIssue: `https://gatsby.dev/lazy-images-feedback`,
     noCI: true,
     testFitness: (): fitnessEnum => {
-      // Take a 10% of slice of users.
-      if (sampleSiteForExperiment(`QUERY_ON_DEMAND`, 10)) {
-        const semverConstraints = {
-          // Because of this, this flag will never show up
-          "gatsby-plugin-sharp": `>=2.10.0`,
-        }
-        if (satisfiesSemvers(semverConstraints)) {
-          return `OPT_IN`
-        } else {
-          // gatsby-plugi-sharp is either not installed or not new enough so
-          // just disable — it won't work anyways.
-          return false
-        }
+      const semverConstraints = {
+        // Because of this, this flag will never show up
+        "gatsby-plugin-sharp": `>=2.10.0`,
+      }
+      if (satisfiesSemvers(semverConstraints)) {
+        return `LOCKED_IN`
       } else {
-        return true
+        // gatsby-plugin-sharp is either not installed or not new enough so
+        // just disable — it won't work anyways.
+        return false
       }
     },
   },
diff --git a/packages/gatsby/src/utils/handle-flags.ts b/packages/gatsby/src/utils/handle-flags.ts
