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

@@ -0,0 +1,5 @@
+---
+"@preact/signals-core": patch
+---
+
+Change effect callback return type from `void` to `unknown`. Same for effect cleanup function.

.github/workflows/main.yml

@@ -45,6 +45,9 @@ jobs:
       - name: Build
         run: pnpm build
 
+      - name: Lint
+        run: pnpm lint
+
       - name: Tests
         run: pnpm test
 

docs/tsconfig.json

@@ -4,5 +4,6 @@
     "jsx": "react-jsx",
     "jsxImportSource": "preact",
     "module": "esnext"
-  }
+  },
+  "exclude": ["node_modules", "dist"]
 }

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",

packages/core/src/index.ts

@@ -669,22 +669,23 @@ 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;
 	_notify(): void;
 	_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();

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++);
 

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", () => {

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(" ");
 		}

packages/react/test/node/setup.js → 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";
 

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/**"]
 }