From f19c8072b4a8e37ff899b478c1e69212c14b40dd Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 29 Dec 2020 21:31:19 +0100 Subject: [PATCH] chore(gatsby): enable query on demand (and lazy images) by default for local development (#28787) * 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 * update snapshots Co-authored-by: Vladimir Razuvaev --- packages/gatsby/src/services/initialize.ts | 25 ++-- .../__snapshots__/handle-flags.ts.snap | 4 +- .../src/utils/__tests__/handle-flags.ts | 125 ++++++++++++++++++ packages/gatsby/src/utils/flags.ts | 92 +++++-------- packages/gatsby/src/utils/handle-flags.ts | 44 ++++-- 5 files changed, 203 insertions(+), 87 deletions(-) diff --git a/packages/gatsby/src/services/initialize.ts b/packages/gatsby/src/services/initialize.ts index 8fb051de241de..4c90d0aa4e95c 100644 --- 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 index b06319b9864bd..7342414ca8070 100644 --- 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 index e95f919a09e74..5298dc0fa8097 100644 --- 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 index 45f89746b96fd..c7fdc438021f7 100644 --- 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 umbrellaIssue?: string @@ -70,8 +79,6 @@ const activeFlags: Array = [ 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 = [ 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 index 3a6bc8f72a89f..5c7799d2456d4 100644 --- a/packages/gatsby/src/utils/handle-flags.ts +++ b/packages/gatsby/src/utils/handle-flags.ts @@ -67,17 +67,27 @@ const handleFlags = ( // Test flags to see if it wants opted in. const optedInFlags = new Map() const applicableFlags = new Map() + const lockedInFlags = new Map() + const lockedInFlagsThatAreInConfig = new Map() availableFlags.forEach(flag => { const fitness = flag.testFitness(flag) - // if user didn't explicitly set a flag (either true or false) - // and it qualifies for auto opt-in - add it to optedInFlags - if (typeof configFlags[flag.name] === `undefined` && fitness === `OPT_IN`) { + const flagIsSetInConfig = typeof configFlags[flag.name] !== `undefined` + + if (fitness === `LOCKED_IN`) { + enabledConfigFlags.push(flag) + lockedInFlags.set(flag.name, flag) + if (flagIsSetInConfig) { + lockedInFlagsThatAreInConfig.set(flag.name, flag) + } + } else if (!flagIsSetInConfig && fitness === `OPT_IN`) { + // if user didn't explicitly set a flag (either true or false) + // and it qualifies for auto opt-in - add it to optedInFlags enabledConfigFlags.push(flag) optedInFlags.set(flag.name, flag) } - if (fitness) { + if (fitness === true || fitness === `OPT_IN`) { applicableFlags.set(flag.name, flag) } }) @@ -133,17 +143,33 @@ const handleFlags = ( let message = `` // Create message about what flags are active. if (enabledConfigFlags.length > 0) { - if (enabledConfigFlags.length - optedInFlags.size > 0) { + if ( + enabledConfigFlags.length - optedInFlags.size - lockedInFlags.size > + 0 + ) { message = `The following flags are active:` enabledConfigFlags.forEach(flag => { - if (!optedInFlags.has(flag.name)) { + if (!optedInFlags.has(flag.name) && !lockedInFlags.has(flag.name)) { message += generateFlagLine(flag) } }) } + if (lockedInFlagsThatAreInConfig.size > 0) { + if (message.length > 0) { + message += `\n\n` + } + message += `Some features you configured with flags are used natively now. +Those flags no longer have any effect and you can remove them from config:` + lockedInFlagsThatAreInConfig.forEach(flag => { + message += generateFlagLine(flag) + }) + } + if (optedInFlags.size > 0) { - message += `\n\n` + if (message.length > 0) { + message += `\n\n` + } 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. @@ -178,7 +204,9 @@ The following flags were automatically enabled on your site:` }) } - message += `\n` + if (message.length > 0) { + message += `\n` + } } return {