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.

Sign up for GitHub

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

chore: Properly typecheck gatsby #31519

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bf79526
failing test
LekoArts May 21, 2021
5b9bde1
wip
LekoArts May 21, 2021
eeafe22
more wip
LekoArts May 21, 2021
2cbb2da
fix(gatsby): fix function types
KyleAMathews May 21, 2021
add4e34
Fix lint
KyleAMathews May 21, 2021
78cb31e
more stuff
LekoArts May 25, 2021
894bdf9
be done maybe
LekoArts May 25, 2021
c2b1b7e
revert path-to-regexp change
LekoArts May 26, 2021
bc61bf0
Merge branch 'master' into proper-ts-checking
LekoArts May 26, 2021
e871e54
fix typecheck after merge
LekoArts May 26, 2021
5f9743e
adding some types
LekoArts May 27, 2021
284b10f
typo
LekoArts May 27, 2021
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/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
"opentracing": "^0.14.4",
"p-defer": "^3.0.0",
"parseurl": "^1.3.3",
"path-to-regexp": "0.1.7",
"path-to-regexp": "^6.2.0",
"physical-cpu-count": "^2.0.0",
"platform": "^1.3.6",
"pnp-webpack-plugin": "^1.6.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby/src/bootstrap/schema-hot-reloader.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions packages/gatsby/src/commands/develop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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
13 changes: 7 additions & 6 deletions packages/gatsby/src/commands/serve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import { match as reachMatch } from "@gatsbyjs/reach-router/lib/utils"
import onExit from "signal-exit"
import report from "gatsby-cli/lib/reporter"
import multer from "multer"
import pathToRegexp from "path-to-regexp"
import { pathToRegexp, Key } 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
Expand Down Expand Up @@ -120,10 +120,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 +134,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 +161,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<Key> = []
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