Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Dec 29, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions packages/gatsby/src/services/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,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`
Comment on lines +46 to +47
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just "random" thing I spotted when looking for qod related changes I needed - we added those to FAST_DEV config flag but not to GATSBY_EXPERIMENTAL_FAST_DEV env var route - I can drop those from here, but it would be weird for FAST_DEV to only map to DEV_SSR 🤷


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`)
Expand Down Expand Up @@ -282,14 +281,10 @@ 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`)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines -88 to -90
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Result of adding checks before appending newlines to message in handle-flags.ts - it should no longer have "ghost" empty lines in front of messages

"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:
Expand Down
125 changes: 125 additions & 0 deletions packages/gatsby/src/utils/__tests__/handle-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 now only and not configurable behavior.
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 now only and not configurable behavior.
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
"
`)
})
})
})
82 changes: 25 additions & 57 deletions packages/gatsby/src/utils/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ 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
Expand All @@ -47,13 +47,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
Expand All @@ -70,8 +77,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`,
],
Expand All @@ -96,39 +101,7 @@ const activeFlags: Array<IFlag> = [
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`,
Expand All @@ -140,21 +113,16 @@ const activeFlags: Array<IFlag> = [
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
}
},
},
Expand Down
44 changes: 36 additions & 8 deletions packages/gatsby/src/utils/handle-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,27 @@ const handleFlags = (
// Test flags to see if it wants opted in.
const optedInFlags = new Map<string, IFlag>()
const applicableFlags = new Map<string, IFlag>()
const lockedInFlags = new Map<string, IFlag>()
const lockedInFlagsThatAreInConfig = new Map<string, IFlag>()
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)
}
})
Expand Down Expand Up @@ -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 now only and not configurable behavior.
Those flags no longer have any effect and you can remove them from config:`
pieh marked this conversation as resolved.
Show resolved Hide resolved
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.

Expand Down Expand Up @@ -178,7 +204,9 @@ The following flags were automatically enabled on your site:`
})
}

message += `\n`
if (message.length > 0) {
message += `\n`
}
Comment on lines +207 to +209
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was sometimes causing \n message without any text (yikes)

}

return {
Expand Down