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: Properly typecheck gatsby #31519

Merged
merged 12 commits into from
May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/gatsby/src/bootstrap/schema-hot-reloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const maybeRebuildSchema = debounce(async (): Promise<void> => {
const activity = report.activityTimer(`rebuild schema`)
activity.start()
await rebuild({ parentSpan: activity })
await updateStateAndRunQueries(false, { parentSpan: activity })
await updateStateAndRunQueries(false, { parentSpan: activity.span })
vladar marked this conversation as resolved.
Show resolved Hide resolved
activity.end()
}, 1000)

Expand Down
21 changes: 14 additions & 7 deletions packages/gatsby/src/cache/cache-fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ exports.create = function (args): typeof DiskStore {
return new DiskStore(args && args.options ? args.options : args)
}

function DiskStore(options): void {
function DiskStore(this: any, options): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes TS2683: 'this' implicitly has type 'any' because it does not have a type annotation. throughout the file

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL: about typing this as first param :O

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this equal to DiskStore? (I'm also new to this in typescript 😂)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this was about having it implicitly:

TS2683: 'this' implicitly has type 'any' because it does not have a type annotation.

I didn't want to type everything so this is the best middleground for now

options = options || {}

this.options = {
Expand Down Expand Up @@ -91,7 +91,12 @@ function DiskStore(options): void {
* @param {function} [cb]
* @returns {Promise}
*/
DiskStore.prototype.set = wrapCallback(async function (key, val, options) {
DiskStore.prototype.set = wrapCallback(async function (
this: any,
key,
val,
options
) {
key = key + ``
const filePath = this._getFilePathByKey(key)

Expand Down Expand Up @@ -128,7 +133,7 @@ DiskStore.prototype.set = wrapCallback(async function (key, val, options) {
* @param {function} [cb]
* @returns {Promise}
*/
DiskStore.prototype.get = wrapCallback(async function (key) {
DiskStore.prototype.get = wrapCallback(async function (this: any, key) {
key = key + ``
const filePath = this._getFilePathByKey(key)

Expand Down Expand Up @@ -172,7 +177,7 @@ DiskStore.prototype.get = wrapCallback(async function (key) {
/**
* delete entry from cache
*/
DiskStore.prototype.del = wrapCallback(async function (key) {
DiskStore.prototype.del = wrapCallback(async function (this: any, key) {
const filePath = this._getFilePathByKey(key)
try {
if (this.options.subdirs) {
Expand All @@ -196,7 +201,9 @@ DiskStore.prototype.del = wrapCallback(async function (key) {
/**
* cleanup cache on disk -> delete all files from the cache
*/
DiskStore.prototype.reset = wrapCallback(async function (): Promise<void> {
DiskStore.prototype.reset = wrapCallback(async function (
this: any
): Promise<void> {
const readdir = promisify(fs.readdir)
const stat = promisify(fs.stat)
const unlink = promisify(fs.unlink)
Expand Down Expand Up @@ -265,10 +272,10 @@ function innerLock(resolve, reject, filePath): void {
* unlocks a file path
* @type {Function}
* @param {string} filePath
* @returns {Promise}
* @returns {void}
* @private
*/
DiskStore.prototype._unlock = function _unlock(filePath): Promise<void> {
DiskStore.prototype._unlock = function _unlock(filePath): void {
globalGatsbyCacheLock.delete(filePath)
}

Expand Down
13 changes: 9 additions & 4 deletions packages/gatsby/src/cache/json-file-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@ const promisify = require(`util`).promisify
const fs = require(`fs`)
const zlib = require(`zlib`)

interface IExternalBuffer {
index: number
buffer: Buffer
}

exports.write = async function (path, data, options): Promise<void> {
const externalBuffers = []
let dataString = JSON.stringify(data, function replacerFunction(k, value) {
const externalBuffers: Array<IExternalBuffer> = []
let dataString = JSON.stringify(data, function replacerFunction(_k, value) {
// Buffers searilize to {data: [...], type: "Buffer"}
if (
value &&
Expand Down Expand Up @@ -103,10 +108,10 @@ exports.read = async function (path, options): Promise<string> {
)
}

const externalBuffers = []
const externalBuffers: Array<IExternalBuffer> = []
let data
try {
data = JSON.parse(dataString, function bufferReceiver(k, value) {
data = JSON.parse(dataString, function bufferReceiver(_k, value) {
if (value && value.type === `Buffer` && value.data) {
return Buffer.from(value.data)
} else if (
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby/src/cache/wrap-callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ module.exports = function wrapCallback<T>(
cb = args.pop()
}

// eslint-disable-next-line @babel/no-invalid-this
const promise = fn.apply(this, args)
// @ts-ignore - unsure if fixing this introduces problems
const promise = fn.apply(this, args) // eslint-disable-line @babel/no-invalid-this
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't eslint-disable-next-line still work because next line is also a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it didn't work for me so I had to move it :/


if (typeof cb === `function`) {
promise.then(
Expand Down
6 changes: 4 additions & 2 deletions packages/gatsby/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ module.exports = async function build(program: IBuildArgs): Promise<void> {

telemetry.trackCli(`BUILD_START`)
signalExit(exitCode => {
telemetry.trackCli(`BUILD_END`, { exitCode })
telemetry.trackCli(`BUILD_END`, {
exitCode: exitCode as number | undefined,
})
})

const buildSpan = buildActivity.span
Expand Down Expand Up @@ -196,7 +198,7 @@ module.exports = async function build(program: IBuildArgs): Promise<void> {
{ parentSpan: buildSpan }
)
buildSSRBundleActivityProgress.start()
let pageRenderer: string
let pageRenderer = ``
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Used before initialized error" - setting a default here will also assign it string

let waitForCompilerCloseBuildHtml
try {
const result = await buildRenderer(program, Stage.BuildHTML, buildSpan)
Expand Down
22 changes: 17 additions & 5 deletions packages/gatsby/src/commands/develop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
} from "gatsby-core-utils"
import reporter from "gatsby-cli/lib/reporter"
import { getSslCert } from "../utils/get-ssl-cert"
import { startDevelopProxy } from "../utils/develop-proxy"
import { IProxyControls, startDevelopProxy } from "../utils/develop-proxy"
import { IProgram, IDebugInfo } from "./types"

// Adapted from https://stackoverflow.com/a/16060619
Expand Down Expand Up @@ -295,7 +295,7 @@ module.exports = async (program: IProgram): Promise<void> => {
null
)

let unlocks: Array<UnlockFn> = []
let unlocks: Array<UnlockFn | null> = []
pieh marked this conversation as resolved.
Show resolved Hide resolved
if (!isCI()) {
const statusUnlock = await createServiceLock(
program.directory,
Expand Down Expand Up @@ -416,7 +416,7 @@ module.exports = async (program: IProgram): Promise<void> => {
)

const files = [rootFile(`gatsby-config.js`), rootFile(`gatsby-node.js`)]
let watcher: chokidar.FSWatcher = null
let watcher: chokidar.FSWatcher

if (!isCI()) {
watcher = chokidar.watch(files).on(`change`, filePath => {
Expand Down Expand Up @@ -498,6 +498,16 @@ module.exports = async (program: IProgram): Promise<void> => {
)
})
}

interface IShutdownServicesOptions {
statusServer: https.Server | http.Server
developProcess: ControllableScript
proxy: IProxyControls
unlocks: Array<UnlockFn | null>
watcher: chokidar.FSWatcher
telemetryServerProcess: ControllableScript
}

function shutdownServices(
{
statusServer,
Expand All @@ -506,7 +516,7 @@ function shutdownServices(
unlocks,
watcher,
telemetryServerProcess,
},
}: IShutdownServicesOptions,
signal: NodeJS.Signals
): Promise<void> {
const services = [
Expand All @@ -518,7 +528,9 @@ function shutdownServices(
]

unlocks.forEach(unlock => {
services.push(unlock())
if (unlock) {
services.push(unlock())
}
})

return Promise.all(services)
Expand Down
19 changes: 14 additions & 5 deletions packages/gatsby/src/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,28 @@ import report from "gatsby-cli/lib/reporter"
import multer from "multer"
import pathToRegexp from "path-to-regexp"
import cookie from "cookie"

import telemetry from "gatsby-telemetry"

import { detectPortInUseAndPrompt } from "../utils/detect-port-in-use-and-prompt"
import { getConfigFile } from "../bootstrap/get-config-file"
import { preferDefault } from "../bootstrap/prefer-default"
import { IProgram } from "./types"
import { IPreparedUrls, prepareUrls } from "../utils/prepare-urls"
import { IGatsbyFunction } from "../redux/types"

interface IMatchPath {
path: string
matchPath: string
}

interface IPathToRegexpKey {
name: string | number
prefix: string
suffix: string
pattern: string
modifier: string
}

interface IServeProgram extends IProgram {
prefixPaths: boolean
}
Expand Down Expand Up @@ -120,10 +128,10 @@ module.exports = async (program: IServeProgram): Promise<void> => {
`functions`
)

let functions
let functions: Array<IGatsbyFunction> = []
try {
functions = JSON.parse(
fs.readFileSync(path.join(compiledFunctionsDir, `manifest.json`))
fs.readFileSync(path.join(compiledFunctionsDir, `manifest.json`), `utf-8`)
pieh marked this conversation as resolved.
Show resolved Hide resolved
)
} catch (e) {
// ignore
Expand All @@ -134,7 +142,7 @@ module.exports = async (program: IServeProgram): Promise<void> => {
`/api/*`,
multer().any(),
express.urlencoded({ extended: true }),
(req, res, next) => {
(req, _, next) => {
const cookies = req.headers.cookie

if (!cookies) {
Expand All @@ -161,12 +169,13 @@ module.exports = async (program: IServeProgram): Promise<void> => {
// We loop until we find the first match.
functions.some(f => {
let exp
const keys = []
const keys: Array<IPathToRegexpKey> = []
if (f.matchPath) {
exp = pathToRegexp(f.matchPath, keys)
}
if (exp && exp.exec(pathFragment) !== null) {
functionObj = f
// @ts-ignore - TS bug? https://stackoverflow.com/questions/50234481/typescript-2-8-3-type-must-have-a-symbol-iterator-method-that-returns-an-iterato
const matches = [...pathFragment.match(exp)].slice(1)
const newParams = {}
matches.forEach(
Expand Down
Loading