Interactivity: Strict type checking (#59865)

Enable `strictNullChecks` and improve types.

Co-authored-by: sirreal <[email protected]>
Co-authored-by: cbravobernal <[email protected]>

Author: sirreal

packages/interactivity/CHANGELOG.md

@@ -2,10 +2,13 @@
 
 ## Unreleased
 
+### Enhancements
+
+-   Strict type checking: fix some missing nulls in published types ([#59865](https://github.com/WordPress/gutenberg/pull/59865/)).
+
 ### Bug Fixes
 
 -	Allow multiple event handlers for the same type with `data-wp-on-document` and `data-wp-on-window`. ([#61009](https://github.com/WordPress/gutenberg/pull/61009))
-
 -	Prevent wrong written directives from killing the runtime ([#61249](https://github.com/WordPress/gutenberg/pull/61249))
 -	Prevent empty namespace or different namespaces from killing the runtime ([#61409](https://github.com/WordPress/gutenberg/pull/61409))
 

packages/interactivity/src/directives.js renamed to packages/interactivity/src/directives.tsx

@@ -1,11 +1,14 @@
+// eslint-disable-next-line eslint-comments/disable-enable-pair
+/* eslint-disable react-hooks/exhaustive-deps */
+
 /* @jsx createElement */
 
 /**
  * External dependencies
  */
-import { h as createElement } from 'preact';
+import { h as createElement, type RefObject } from 'preact';
 import { useContext, useMemo, useRef } from 'preact/hooks';
-import { deepSignal, peek } from 'deepsignal';
+import { deepSignal, peek, type DeepSignal } from 'deepsignal';
 
 /**
  * Internal dependencies
@@ -23,8 +26,8 @@ const contextObjectToProxy = new WeakMap();
 const contextProxyToObject = new WeakMap();
 const contextObjectToFallback = new WeakMap();
 
-const isPlainObject = ( item ) =>
-	item && typeof item === 'object' && item.constructor === Object;
+const isPlainObject = ( item: unknown ): boolean =>
+	Boolean( item && typeof item === 'object' && item.constructor === Object );
 
 const descriptor = Reflect.getOwnPropertyDescriptor;
 
@@ -37,17 +40,17 @@ const descriptor = Reflect.getOwnPropertyDescriptor;
  * By default, all plain objects inside the context are wrapped, unless it is
  * listed in the `ignore` option.
  *
- * @param {Object} current   Current context.
- * @param {Object} inherited Inherited context, used as fallback.
+ * @param current   Current context.
+ * @param inherited Inherited context, used as fallback.
  *
- * @return {Object} The wrapped context object.
+ * @return The wrapped context object.
  */
-const proxifyContext = ( current, inherited = {} ) => {
+const proxifyContext = ( current: object, inherited: object = {} ): object => {
 	// Update the fallback object reference when it changes.
 	contextObjectToFallback.set( current, inherited );
 	if ( ! contextObjectToProxy.has( current ) ) {
 		const proxy = new Proxy( current, {
-			get: ( target, k ) => {
+			get: ( target: DeepSignal< any >, k ) => {
 				const fallback = contextObjectToFallback.get( current );
 				// Always subscribe to prop changes in the current context.
 				const currentProp = target[ k ];
@@ -127,10 +130,13 @@ const proxifyContext = ( current, inherited = {} ) => {
 /**
  * Recursively update values within a deepSignal object.
  *
- * @param {Object} target A deepSignal instance.
- * @param {Object} source Object with properties to update in `target`
+ * @param target A deepSignal instance.
+ * @param source Object with properties to update in `target`.
  */
-const updateSignals = ( target, source ) => {
+const updateSignals = (
+	target: DeepSignal< any >,
+	source: DeepSignal< any >
+) => {
 	for ( const k in source ) {
 		if (
 			isPlainObject( peek( target, k ) ) &&
@@ -146,23 +152,23 @@ const updateSignals = ( target, source ) => {
 /**
  * Recursively clone the passed object.
  *
- * @param {Object} source Source object.
- * @return {Object} Cloned object.
+ * @param source Source object.
+ * @return Cloned object.
  */
-const deepClone = ( source ) => {
+function deepClone< T >( source: T ): T {
 	if ( isPlainObject( source ) ) {
 		return Object.fromEntries(
-			Object.entries( source ).map( ( [ key, value ] ) => [
+			Object.entries( source as object ).map( ( [ key, value ] ) => [
 				key,
 				deepClone( value ),
 			] )
-		);
+		) as T;
 	}
 	if ( Array.isArray( source ) ) {
-		return source.map( ( i ) => deepClone( i ) );
+		return source.map( ( i ) => deepClone( i ) ) as T;
 	}
 	return source;
-};
+}
 
 const newRule =
 	/(?:([\u0080-\uFFFF\w-%@]+) *:? *([^{;]+?);|([^;}{]*?) *{)|(}\s*)/g;
@@ -176,10 +182,12 @@ const empty = ' ';
  * Made by Cristian Bote (@cristianbote) for Goober.
  * https://unpkg.com/browse/[email protected]/src/core/astish.js
  *
- * @param {string} val CSS string.
- * @return {Object} CSS object.
+ * @param val CSS string.
+ * @return CSS object.
  */
-const cssStringToObject = ( val ) => {
+const cssStringToObject = (
+	val: string
+): Record< string, string | number > => {
 	const tree = [ {} ];
 	let block, left;
 
@@ -203,10 +211,9 @@ const cssStringToObject = ( val ) => {
  * Creates a directive that adds an event listener to the global window or
  * document object.
  *
- * @param {string} type 'window' or 'document'
- * @return {void}
+ * @param type 'window' or 'document'
  */
-const getGlobalEventDirective = ( type ) => {
+const getGlobalEventDirective = ( type: 'window' | 'document' ) => {
 	return ( { directives, evaluate } ) => {
 		directives[ `on-${ type }` ]
 			.filter( ( { suffix } ) => suffix !== 'default' )
@@ -217,7 +224,7 @@ const getGlobalEventDirective = ( type ) => {
 					const globalVar = type === 'window' ? window : document;
 					globalVar.addEventListener( eventName, cb );
 					return () => globalVar.removeEventListener( eventName, cb );
-				}, [] );
+				} );
 			} );
 	};
 };
@@ -333,9 +340,13 @@ export default () => {
 						 * need deps because it only needs to do it the first time.
 						 */
 						if ( ! result ) {
-							element.ref.current.classList.remove( className );
+							(
+								element.ref as RefObject< HTMLElement >
+							 ).current!.classList.remove( className );
 						} else {
-							element.ref.current.classList.add( className );
+							(
+								element.ref as RefObject< HTMLElement >
+							 ).current!.classList.add( className );
 						}
 					} );
 				} );
@@ -368,9 +379,13 @@ export default () => {
 					 * because it only needs to do it the first time.
 					 */
 					if ( ! result ) {
-						element.ref.current.style.removeProperty( styleProp );
+						(
+							element.ref as RefObject< HTMLElement >
+						 ).current!.style.removeProperty( styleProp );
 					} else {
-						element.ref.current.style[ styleProp ] = result;
+						(
+							element.ref as RefObject< HTMLElement >
+						 ).current!.style[ styleProp ] = result;
 					}
 				} );
 			} );
@@ -390,7 +405,8 @@ export default () => {
 				 * first time. After that, Preact will handle the changes.
 				 */
 				useInit( () => {
-					const el = element.ref.current;
+					const el = ( element.ref as RefObject< HTMLElement > )
+						.current!;
 
 					/*
 					 * We set the value directly to the corresponding HTMLElement instance
@@ -462,6 +478,8 @@ export default () => {
 				type: Type,
 				props: { innerHTML, ...rest },
 			},
+		}: {
+			element: any;
 		} ) => {
 			// Preserve the initial inner HTML.
 			const cached = useMemo( () => innerHTML, [] );
@@ -477,6 +495,11 @@ export default () => {
 	// data-wp-text
 	directive( 'text', ( { directives: { text }, element, evaluate } ) => {
 		const entry = text.find( ( { suffix } ) => suffix === 'default' );
+		if ( ! entry ) {
+			element.props.children = null;
+			return;
+		}
+
 		try {
 			const result = evaluate( entry );
 			element.props.children =

packages/interactivity/src/hooks.tsx

@@ -1,5 +1,8 @@
 /* @jsx createElement */
 
+// eslint-disable-next-line eslint-comments/disable-enable-pair
+/* eslint-disable react-hooks/exhaustive-deps */
+
 /**
  * External dependencies
  */
@@ -8,6 +11,7 @@ import {
 	options,
 	createContext,
 	cloneElement,
+	type ComponentChildren,
 } from 'preact';
 import { useRef, useCallback, useContext } from 'preact/hooks';
 import type { VNode, Context, RefObject } from 'preact';
@@ -18,7 +22,7 @@ import type { VNode, Context, RefObject } from 'preact';
 import { store, stores, universalUnlock } from './store';
 import { warn } from './utils/warn';
 interface DirectiveEntry {
-	value: string | Object;
+	value: string | object;
 	namespace: string;
 	suffix: string;
 }
@@ -33,11 +37,15 @@ interface DirectiveArgs {
 	/**
 	 * Props present in the current element.
 	 */
-	props: Object;
+	props: { children?: ComponentChildren };
 	/**
 	 * Virtual node representing the element.
 	 */
-	element: VNode;
+	element: VNode< {
+		class?: string;
+		style?: string | Record< string, string | number >;
+		content?: ComponentChildren;
+	} >;
 	/**
 	 * The inherited context.
 	 */
@@ -50,7 +58,7 @@ interface DirectiveArgs {
 }
 
 interface DirectiveCallback {
-	( args: DirectiveArgs ): VNode | void;
+	( args: DirectiveArgs ): VNode | null | void;
 }
 
 interface DirectiveOptions {
@@ -65,7 +73,7 @@ interface DirectiveOptions {
 
 interface Scope {
 	evaluate: Evaluate;
-	context: Context< any >;
+	context: object;
 	ref: RefObject< HTMLElement >;
 	attributes: createElement.JSX.HTMLAttributes;
 }
@@ -102,7 +110,7 @@ const immutableError = () => {
 		'Please use `data-wp-bind` to modify the attributes of an element.'
 	);
 };
-const immutableHandlers = {
+const immutableHandlers: ProxyHandler< object > = {
 	get( target, key, receiver ) {
 		const value = Reflect.get( target, key, receiver );
 		return !! value && typeof value === 'object'
@@ -112,7 +120,7 @@ const immutableHandlers = {
 	set: immutableError,
 	deleteProperty: immutableError,
 };
-const deepImmutable = < T extends Object = {} >( target: T ): T => {
+const deepImmutable = < T extends object = {} >( target: T ): T => {
 	if ( ! immutableMap.has( target ) ) {
 		immutableMap.set( target, new Proxy( target, immutableHandlers ) );
 	}
@@ -260,7 +268,7 @@ export const directive = (
 };
 
 // Resolve the path to some property of the store object.
-const resolve = ( path, namespace ) => {
+const resolve = ( path: string, namespace: string ) => {
 	if ( ! namespace ) {
 		warn(
 			`The "namespace" cannot be "{}", "null" or an empty string. Path: ${ path }`

packages/interactivity/src/store.ts

@@ -17,7 +17,7 @@ import {
 } from './hooks';
 
 const isObject = ( item: unknown ): item is Record< string, unknown > =>
-	item && typeof item === 'object' && item.constructor === Object;
+	Boolean( item && typeof item === 'object' && item.constructor === Object );
 
 const deepMerge = ( target: any, source: any ) => {
 	if ( isObject( target ) && isObject( source ) ) {
@@ -338,12 +338,12 @@ export const populateInitialData = ( data?: {
 	config?: Record< string, unknown >;
 } ) => {
 	if ( isObject( data?.state ) ) {
-		Object.entries( data.state ).forEach( ( [ namespace, state ] ) => {
+		Object.entries( data!.state ).forEach( ( [ namespace, state ] ) => {
 			store( namespace, { state }, { lock: universalUnlock } );
 		} );
 	}
 	if ( isObject( data?.config ) ) {
-		Object.entries( data.config ).forEach( ( [ namespace, config ] ) => {
+		Object.entries( data!.config ).forEach( ( [ namespace, config ] ) => {
 			storeConfigs.set( namespace, config );
 		} );
 	}

packages/interactivity/src/utils.ts

@@ -62,8 +62,8 @@ const afterNextFrame = ( callback: () => void ) => {
  * @return The Flusher object with `flush` and `dispose` properties.
  */
 function createFlusher( compute: () => unknown, notify: () => void ): Flusher {
-	let flush: () => void;
-	const dispose = effect( function () {
+	let flush: () => void = () => undefined;
+	const dispose = effect( function ( this: any ) {
 		flush = this.c.bind( this );
 		this.x = compute;
 		this.c = notify;
@@ -82,7 +82,7 @@ function createFlusher( compute: () => unknown, notify: () => void ): Flusher {
  */
 export function useSignalEffect( callback: () => unknown ) {
 	_useEffect( () => {
-		let eff = null;
+		let eff: Flusher | null = null;
 		let isExecuting = false;
 
 		const notify = async () => {
@@ -273,7 +273,7 @@ export const createRootFragment = (
 	parent: Element,
 	replaceNode: Node | Node[]
 ) => {
-	replaceNode = [].concat( replaceNode );
+	replaceNode = ( [] as Node[] ).concat( replaceNode );
 	const sibling = replaceNode[ replaceNode.length - 1 ].nextSibling;
 	function insert( child: any, root: any ) {
 		parent.insertBefore( child, root || sibling );