Skip to content

Commit

Permalink
feature(gatsby): Add gatsby feedback (#21841)
Browse files Browse the repository at this point in the history
* Create feedback command

* progress with prompting heuristics

* Finish heuristic

* Finish hueristics and add tests

* start hooking up feedback to cli commands

* Finish implementation

* Add an additional random selection heuristic

* update message

* add try/catch

* Add documentation

* Update docs/docs/feedback.md

Co-Authored-By: LB <laurie@gatsbyjs.com>

* Update docs/docs/feedback.md

Co-Authored-By: LB <laurie@gatsbyjs.com>

* fix feedback cli issues

* rename files

* fix lint

* disable code to see if CI passes

* try to run with more memory

* pass tests

Co-authored-by: LB <laurie@gatsbyjs.com>
  • Loading branch information
blainekasten and LB authored Mar 6, 2020
1 parent 83ae63d commit 096c28d
Show file tree
Hide file tree
Showing 17 changed files with 371 additions and 16 deletions.
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ aliases:
- <<: *install_node_modules
- run: yarn list react
- run: ./scripts/assert-changed-files.sh "packages/*|.circleci/*"
- run: yarn jest -w 1 --ci
- run: GATSBY_DB_NODES=loki yarn jest -w 1 --ci
- run: node --max-old-space-size=2048 ./node_modules/.bin/jest -w 1 --ci
- run: GATSBY_DB_NODES=loki node --max-old-space-size=2048 ./node_modules/.bin/jest -w 1 --ci

e2e-test-workflow: &e2e-test-workflow
filters:
Expand Down
9 changes: 9 additions & 0 deletions docs/docs/cli-feedback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
title: Gatsby CLI Feedback
---

Gatsby users will be prompted every 3 months (at the most) to give feedback to Gatsby on our products and services. This feedback is incredibly valuable and will help us shape Gatsby.

## How to opt-out

Users may always opt-out from the feedback prompts with `gatsby feedback --disable` or by setting the environment variable `GATSBY_TELEMETRY_DISABLED` to `1`
25 changes: 19 additions & 6 deletions packages/gatsby-cli/src/create-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function buildLocalCommands(cli, isLocalSite) {
? [`> 1%`, `last 2 versions`, `IE >= 9`]
: [`>0.25%`, `not dead`]

let siteInfo = { directory, browserslist: DEFAULT_BROWSERS }
const siteInfo = { directory, browserslist: DEFAULT_BROWSERS }
const useYarn = existsSync(path.join(directory, `yarn.lock`))
if (isLocalSite) {
const json = require(path.join(directory, `package.json`))
Expand Down Expand Up @@ -94,8 +94,8 @@ function buildLocalCommands(cli, isLocalSite) {
process.env.gatsby_executing_command = command
report.verbose(`set gatsby_executing_command: "${command}"`)

let localCmd = resolveLocalCommand(command)
let args = { ...argv, ...siteInfo, report, useYarn, setStore }
const localCmd = resolveLocalCommand(command)
const args = { ...argv, ...siteInfo, report, useYarn, setStore }

report.verbose(`running command: ${command}`)
return handler ? handler(args, localCmd) : localCmd(args)
Expand Down Expand Up @@ -266,6 +266,19 @@ function buildLocalCommands(cli, isLocalSite) {
},
})

cli.command({
command: `feedback`,
builder: _ =>
_.option(`disable`, {
type: `boolean`,
describe: `Opt out of future feedback requests`,
}).option(`enable`, {
type: `boolean`,
describe: `Opt into future feedback requests`,
}),
handler: getCommandHandler(`feedback`),
})

cli.command({
command: `clean`,
desc: `Wipe the local gatsby environment including built assets and cache`,
Expand All @@ -285,7 +298,7 @@ function buildLocalCommands(cli, isLocalSite) {
function isLocalGatsbySite() {
let inGatsbySite = false
try {
let { dependencies, devDependencies } = require(path.resolve(
const { dependencies, devDependencies } = require(path.resolve(
`./package.json`
))
inGatsbySite =
Expand Down Expand Up @@ -317,8 +330,8 @@ Gatsby version: ${gatsbyVersion}
}

module.exports = argv => {
let cli = yargs()
let isLocalSite = isLocalGatsbySite()
const cli = yargs()
const isLocalSite = isLocalGatsbySite()

cli
.scriptName(`gatsby`)
Expand Down
14 changes: 14 additions & 0 deletions packages/gatsby-core-utils/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import ConfigStore from "configstore"

/**
* Encrypts an input using md5 hash of hexadecimal digest.
*
Expand Down Expand Up @@ -49,3 +51,15 @@ export declare function isCI(): boolean
* @return {string | null} The name of the CI if available. Defaults to null if not in CI
*/
export declare function getCIName(): string | null

/**
* Gets the configstore instance related to gatsby
* @return {ConfigStore} the ConfigStore instance for gatsby
*/
export declare function getConfigStore(): ConfigStore

/**
* Gets the version of the installed gatsby from node_modules
* @return {string} the version string of the installed gatsby
*/
export declare function getGatsbyVersion(): string
17 changes: 17 additions & 0 deletions packages/gatsby-core-utils/src/get-config-store.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Configstore from "configstore"

let config

module.exports = () => {
if (!config) {
config = new Configstore(
`gatsby`,
{},
{
globalConfigPath: true,
}
)
}

return config
}
14 changes: 14 additions & 0 deletions packages/gatsby-core-utils/src/get-gatsby-version.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import path from "path"

module.exports = () => {
try {
return require(path.join(
process.cwd(),
`node_modules`,
`gatsby`,
`package.json`
)).version
} catch (e) {
return ``
}
}
2 changes: 2 additions & 0 deletions packages/gatsby-core-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ exports.urlResolve = require(`./url`).resolve
exports.isCI = require(`./ci`).isCI
exports.getCIName = require(`./ci`).getCIName
exports.createRequireFromPath = require(`./create-require-from-path`)
exports.getConfigStore = require(`./get-config-store`)
exports.getGatsbyVersion = require(`./get-gatsby-version`)
2 changes: 2 additions & 0 deletions packages/gatsby/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"core-js": "^2.6.11",
"cors": "^2.8.5",
"css-loader": "^1.0.1",
"date-fns": "^2.10.0",
"debug": "^3.2.6",
"del": "^5.1.0",
"detect-port": "^1.3.0",
Expand Down Expand Up @@ -89,6 +90,7 @@
"jest-worker": "^24.9.0",
"json-loader": "^0.5.7",
"json-stringify-safe": "^5.0.1",
"latest-version": "5.1.0",
"lodash": "^4.17.15",
"lokijs": "^1.5.8",
"md5": "^2.2.1",
Expand Down
8 changes: 8 additions & 0 deletions packages/gatsby/src/commands/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const { structureWebpackErrors } = require(`../utils/webpack-error-utils`)
const {
waitUntilAllJobsComplete: waitUntilAllJobsV2Complete,
} = require(`../utils/jobs-manager`)
import {
userPassesFeedbackRequestHeuristic,
showFeedbackRequest,
} from "../utils/feedback"
const buildUtils = require(`../commands/build-utils`)
const { boundActionCreators } = require(`../redux/actions`)

Expand Down Expand Up @@ -307,4 +311,8 @@ module.exports = async function build(program: BuildArgs) {
report.info(`.cache/deletedPages.txt created`)
}
}

if (await userPassesFeedbackRequestHeuristic()) {
showFeedbackRequest()
}
}
8 changes: 8 additions & 0 deletions packages/gatsby/src/commands/clean.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
const fs = require(`fs-extra`)
const path = require(`path`)
import {
userPassesFeedbackRequestHeuristic,
showFeedbackRequest,
} from "../utils/feedback"

module.exports = async function clean(args) {
const { directory, report } = args
Expand All @@ -13,4 +17,8 @@ module.exports = async function clean(args) {
)

report.info(`Successfully deleted directories`)

if (await userPassesFeedbackRequestHeuristic()) {
showFeedbackRequest()
}
}
17 changes: 17 additions & 0 deletions packages/gatsby/src/commands/develop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ import {
reportWebpackWarnings,
structureWebpackErrors,
} from "../utils/webpack-error-utils"
import {
userPassesFeedbackRequestHeuristic,
showFeedbackRequest,
} from "../utils/feedback"

import { BuildHTMLStage, IProgram } from "./types"
import { waitUntilAllJobsComplete as waitUntilAllJobsV2Complete } from "../utils/jobs-manager"
Expand Down Expand Up @@ -353,6 +357,19 @@ async function startServer(program: IProgram): Promise<IServer> {
}

module.exports = async (program: IProgram): Promise<void> => {
// We want to prompt the feedback request when users quit develop
// assuming they pass the heuristic check to know they are a user
// we want to request feedback from, and we're not annoying them.
process.on(
`SIGINT`,
async (): Promise<void> => {
if (await userPassesFeedbackRequestHeuristic()) {
showFeedbackRequest()
}
process.exit(0)
}
)

if (process.env.GATSBY_EXPERIMENTAL_PAGE_BUILD_ON_DATA_CHANGES) {
report.panic(
`The flag ${chalk.yellow(
Expand Down
30 changes: 30 additions & 0 deletions packages/gatsby/src/commands/feedback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import {
setFeedbackDisabledValue,
showFeedbackRequest,
} from "../utils/feedback"
import { IProgram } from "./types"

// This is because we splat command line arguments onto this object.
// A good refactor would be to put this inside a key like `cliArgs`
interface IFeedbackProgram extends IProgram {
disable?: boolean
enable?: boolean
}

module.exports = async function feedback(
program: IFeedbackProgram
): Promise<void> {
if (program.disable) {
program.report.info(`Disabling gatsby feedback requests`)
setFeedbackDisabledValue(true)
return
}

if (program.enable) {
program.report.info(`Enabling gatsby feedback requests`)
setFeedbackDisabledValue(false)
return
}

showFeedbackRequest()
}
3 changes: 2 additions & 1 deletion packages/gatsby/src/commands/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PackageJson } from "gatsby"
import { PackageJson, Reporter } from "gatsby"

export interface ICert {
keyPath: string
Expand All @@ -13,6 +13,7 @@ export interface IProgram {
openTracingConfigFile: string
port: number
host: string
report: Reporter
[`cert-file`]?: string
[`key-file`]?: string
directory: string
Expand Down
115 changes: 115 additions & 0 deletions packages/gatsby/src/utils/__tests__/feedback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import { getConfigStore, getGatsbyVersion } from "gatsby-core-utils"
import {
setFeedbackDisabledValue,
userPassesFeedbackRequestHeuristic,
} from "../feedback"
jest.mock(`date-fns/getDayOfYear`, (): (() => number) => (): number => {
// This is required for Hueristic 1 to always match up
// When Math.random returns 1 (mocked in the `clearStateToAllowHeuristicsToPass` fn)
const currentQuarter = Math.floor((new Date().getMonth() + 3) / 3)
return 1 * 30 * 3 * currentQuarter
})
jest.mock(`gatsby-core-utils`, () => {
return {
...jest.requireActual(`gatsby-core-utils`),
getGatsbyVersion: jest.fn(() => `2.1.1`),
}
})

jest.mock(`latest-version`, (): (() => Promise<string>) => (): Promise<
string
> => Promise.resolve(`2.1.1`))

const dateFromSixMonthsAgo = new Date()
dateFromSixMonthsAgo.setMonth(dateFromSixMonthsAgo.getMonth() - 6)
const mathRandom = Math.random

// This function resets all state to make the heuristic
// checks all pass. This is to be used to make sure an individual
// test truly only gets triggered by the state manipulations
// that exist within that test.
const clearStateToAllowHeuristicsToPass = (): void => {
// Heuristic 1
Math.random = jest.fn(() => 1)
// Heuristic 2
setFeedbackDisabledValue(false)
// Heuristic 3
delete process.env.GATSBY_FEEDBACK_DISABLED
// Heuristic 4
getConfigStore().set(`feedback.lastRequestDate`, dateFromSixMonthsAgo)
// Heuristic 5
;(getGatsbyVersion as jest.Mock).mockReturnValue(`2.1.1`)
}

describe(`feedback`, () => {
describe(`userPassesFeedbackRequestHeuristic returns false when`, () => {
beforeEach(clearStateToAllowHeuristicsToPass)

afterEach(() => {
Math.random = mathRandom
})

it(`Heuristic 1: The random number generator hits today`, async () => {
// ensure default is passing before manipulating a test
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)

// Change the random value to return a different date
;(Math.random as jest.Mock).mockReturnValue(0.5)
expect(await userPassesFeedbackRequestHeuristic()).toBe(false)

// Unset to ensure tests are stable
;(Math.random as jest.Mock).mockReturnValue(1)
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)
})

it(`Heuristic 2: the gatsby disabled key is set to false`, async () => {
// ensure default is passing before manipulating a test
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)

setFeedbackDisabledValue(true)
expect(await userPassesFeedbackRequestHeuristic()).toBe(false)

// Unset to ensure tests are stable
setFeedbackDisabledValue(false)
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)
})

it("Heuristic 3: the process.env.GATSBY_FEEDBACK_DISABLED argument is set to `1`", async () => {
// ensure default is passing before manipulating a test
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)

process.env.GATSBY_FEEDBACK_DISABLED = `1`
expect(await userPassesFeedbackRequestHeuristic()).toBe(false)

// Unset to ensure tests are stable
process.env.GATSBY_FEEDBACK_DISABLED = `0`
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)
})

it(`Heuristic 4: It has been more than 3 months since the last feedback request`, async () => {
// ensure default is passing before manipulating a test
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)

// Set last prompt date to today
getConfigStore().set(`feedback.lastRequestDate`, Date.now())
expect(await userPassesFeedbackRequestHeuristic()).toBe(false)

// Unset to ensure tests are stable
getConfigStore().set(`feedback.lastRequestDate`, dateFromSixMonthsAgo)
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)
})

it(`Heuristic 5: The installed Gatsby is on the latest major + minor version`, async () => {
// ensure default is passing before manipulating a test
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)

// Mock the installed version to be a previous minor to force false
;(getGatsbyVersion as jest.Mock).mockReturnValue(`2.0.0`)
expect(await userPassesFeedbackRequestHeuristic()).toBe(false)

// Unset to ensure tests are stable
;(getGatsbyVersion as jest.Mock).mockReturnValue(`2.1.1`)
expect(await userPassesFeedbackRequestHeuristic()).toBe(true)
})
})
})
Loading

0 comments on commit 096c28d

Please sign in to comment.