From 26f6526875ef0968621c4113594ac95b93de5163 Mon Sep 17 00:00:00 2001 From: Andre Wiggins <459878+andrewiggins@users.noreply.github.com> Date: Fri, 12 May 2023 15:28:47 -0700 Subject: [PATCH] Add type checking to CI (#359) * Fix up tsc config * Setup global expect & sinon for NodeJS tests * Fix tsc errors in src and keep demo errors the same as before * Run tsc and linting in CI * Add changeset for change to signals core --- .changeset/gorgeous-ravens-design.md | 5 +++++ .github/workflows/main.yml | 3 +++ docs/tsconfig.json | 3 ++- package.json | 8 +++++--- packages/core/src/index.ts | 13 +++++++------ packages/core/test/signal.test.tsx | 2 +- .../react/test/node/renderToStaticMarkup.test.tsx | 2 -- packages/react/test/shared/utils.ts | 3 ++- {packages/react/test => test}/node/setup.js | 8 ++++++++ tsconfig.json | 4 +++- 10 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 .changeset/gorgeous-ravens-design.md rename {packages/react/test => test}/node/setup.js (78%) diff --git a/.changeset/gorgeous-ravens-design.md b/.changeset/gorgeous-ravens-design.md new file mode 100644 index 000000000..9a6107d2f --- /dev/null +++ b/.changeset/gorgeous-ravens-design.md @@ -0,0 +1,5 @@ +--- +"@preact/signals-core": patch +--- + +Change effect callback return type from `void` to `unknown`. Same for effect cleanup function. diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fb8f5f5e9..65f3a405b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -45,6 +45,9 @@ jobs: - name: Build run: pnpm build + - name: Lint + run: pnpm lint + - name: Tests run: pnpm test diff --git a/docs/tsconfig.json b/docs/tsconfig.json index 3305ffcf8..10f2e1a43 100644 --- a/docs/tsconfig.json +++ b/docs/tsconfig.json @@ -4,5 +4,6 @@ "jsx": "react-jsx", "jsxImportSource": "preact", "module": "esnext" - } + }, + "exclude": ["node_modules", "dist"] } diff --git a/package.json b/package.json index 92633af23..e73bc36a6 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,9 @@ "postbuild:preact": "cd packages/preact/dist && mv -f preact/src/index.d.ts signals.d.ts && rm -dr preact", "postbuild:react": "cd packages/react/dist && mv -f react/src/index.d.ts signals.d.ts && rm -dr react", "postbuild": "node ./scripts/node-13-exports.js", - "lint": "eslint 'packages/**/*.{ts,tsx,js,jsx}'", + "lint": "pnpm lint:eslint && pnpm lint:tsc", + "lint:eslint": "eslint 'packages/**/*.{ts,tsx,js,jsx}'", + "lint:tsc": "tsc -p tsconfig.json --noEmit", "test": "pnpm test:karma && pnpm test:mocha", "test:minify": "pnpm test:karma:minify && pnpm test:mocha", "test:prod": "pnpm test:karma:prod && pnpm test:mocha:prod", @@ -20,8 +22,8 @@ "test:karma:watch": "karma start karma.conf.js --no-single-run", "test:karma:prod": "cross-env MINIFY=true NODE_ENV=production karma start karma.conf.js --single-run", "test:karma:prod:watch": "cross-env NODE_ENV=production karma start karma.conf.js --no-single-run", - "test:mocha": "cross-env COVERAGE=true mocha --require packages/react/test/node/setup.js --recursive packages/react/test/node/**.test.tsx", - "test:mocha:prod": "cross-env COVERAGE=true NODE_ENV=production mocha --require packages/react/test/node/setup.js --recursive packages/react/test/node/**.test.tsx", + "test:mocha": "cross-env COVERAGE=true mocha --require test/node/setup.js --recursive packages/react/test/node/**.test.tsx", + "test:mocha:prod": "cross-env COVERAGE=true NODE_ENV=production mocha --require test/node/setup.js --recursive packages/react/test/node/**.test.tsx", "docs:start": "cd docs && pnpm start", "docs:build": "cd docs && pnpm build", "docs:preview": "cd docs && pnpm preview", diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e341f16c2..4dacaf374 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -669,14 +669,15 @@ function endEffect(this: Effect, prevContext?: Computed | Effect) { endBatch(); } +type EffectCleanup = () => unknown; declare class Effect { - _compute?: () => void | (() => void); - _cleanup?: () => void; + _compute?: () => unknown | EffectCleanup; + _cleanup?: () => unknown; _sources?: Node; _nextBatchedEffect?: Effect; _flags: number; - constructor(compute: () => void | (() => void)); + constructor(compute: () => unknown | EffectCleanup); _callback(): void; _start(): () => void; @@ -684,7 +685,7 @@ declare class Effect { _dispose(): void; } -function Effect(this: Effect, compute: () => void | (() => void)) { +function Effect(this: Effect, compute: () => unknown | EffectCleanup) { this._compute = compute; this._cleanup = undefined; this._sources = undefined; @@ -700,7 +701,7 @@ Effect.prototype._callback = function () { const cleanup = this._compute(); if (typeof cleanup === "function") { - this._cleanup = cleanup; + this._cleanup = cleanup as EffectCleanup; } } finally { finish(); @@ -738,7 +739,7 @@ Effect.prototype._dispose = function () { } }; -function effect(compute: () => void | (() => void)): () => void { +function effect(compute: () => unknown | EffectCleanup): () => void { const effect = new Effect(compute); try { effect._callback(); diff --git a/packages/core/test/signal.test.tsx b/packages/core/test/signal.test.tsx index 0a86b8a4d..00b5a543f 100644 --- a/packages/core/test/signal.test.tsx +++ b/packages/core/test/signal.test.tsx @@ -873,7 +873,7 @@ describe("computed()", () => { }); it("should disallow setting signal's value", () => { - const v: number = 123; + const v = 123; const a: Signal = signal(v); const c: Signal = computed(() => a.value++); diff --git a/packages/react/test/node/renderToStaticMarkup.test.tsx b/packages/react/test/node/renderToStaticMarkup.test.tsx index 572d4026b..08ad99be9 100644 --- a/packages/react/test/node/renderToStaticMarkup.test.tsx +++ b/packages/react/test/node/renderToStaticMarkup.test.tsx @@ -1,8 +1,6 @@ import { signal, useSignalEffect } from "@preact/signals-react"; -import { expect } from "chai"; import { createElement } from "react"; import { renderToStaticMarkup } from "react-dom/server"; -import sinon from "sinon"; import { mountSignalsTests } from "../shared/mounting"; describe("renderToStaticMarkup", () => { diff --git a/packages/react/test/shared/utils.ts b/packages/react/test/shared/utils.ts index e282deb43..2d561b936 100644 --- a/packages/react/test/shared/utils.ts +++ b/packages/react/test/shared/utils.ts @@ -112,7 +112,8 @@ export function checkConsoleErrorLogs(): void { if (errorSpy.called) { let message: string; if (errorSpy.firstCall.args[0].toString().includes("%s")) { - message = consoleFormat(...errorSpy.firstCall.args); + const firstArg = errorSpy.firstCall.args[0]; + message = consoleFormat(firstArg, ...errorSpy.firstCall.args.slice(1)); } else { message = errorSpy.firstCall.args.join(" "); } diff --git a/packages/react/test/node/setup.js b/test/node/setup.js similarity index 78% rename from packages/react/test/node/setup.js rename to test/node/setup.js index 210f6fac3..8c08f848c 100644 --- a/packages/react/test/node/setup.js +++ b/test/node/setup.js @@ -1,5 +1,13 @@ // import register from "@babel/register"; const register = require("@babel/register").default; +const chai = require("chai"); +const sinon = require("sinon"); +const sinonChai = require("sinon-chai"); + +globalThis.expect = chai.expect; +// @ts-expect-error Not sure why TS isn't picking up the declared sinon global from karma-chai-sinon +globalThis.sinon = sinon; +chai.use(sinonChai); const coverage = String(process.env.COVERAGE) === "true"; diff --git a/tsconfig.json b/tsconfig.json index 043626177..c4290deff 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,10 +10,12 @@ "jsxFragmentFactory": "Fragment", "stripInternal": true, "noUnusedLocals": true, + "noEmit": true, "paths": { "@preact/signals-core": ["./packages/core/src/index.ts"], "@preact/signals": ["./packages/preact/src/index.ts"], "@preact/signals-react": ["./packages/react/src/index.ts"] } - } + }, + "exclude": ["docs", "packages/*/dist/**"] }