From 966f0f709e5d0482ee2bc39c355b51e63de269ec Mon Sep 17 00:00:00 2001 From: Jon Surrell Date: Wed, 15 May 2024 20:50:02 +0200 Subject: [PATCH] Interactivity: Strict type checking (#59865) Enable `strictNullChecks` and improve types. Co-authored-by: sirreal Co-authored-by: cbravobernal --- packages/interactivity/CHANGELOG.md | 5 +- .../src/{directives.js => directives.tsx} | 85 ++++++++++------- packages/interactivity/src/hooks.tsx | 24 +++-- packages/interactivity/src/store.ts | 6 +- packages/interactivity/src/utils.ts | 8 +- packages/interactivity/src/vdom.ts | 92 ++++++++++++------- packages/interactivity/tsconfig.json | 4 +- tsconfig.base.json | 2 +- 8 files changed, 141 insertions(+), 85 deletions(-) rename packages/interactivity/src/{directives.js => directives.tsx} (88%) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index fc22086bef443a..01097504d7064a 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/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)) diff --git a/packages/interactivity/src/directives.js b/packages/interactivity/src/directives.tsx similarity index 88% rename from packages/interactivity/src/directives.js rename to packages/interactivity/src/directives.tsx index 8100ac7a52eee5..07328df8af210b 100644 --- a/packages/interactivity/src/directives.js +++ b/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/goober@2.1.13/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 = diff --git a/packages/interactivity/src/hooks.tsx b/packages/interactivity/src/hooks.tsx index 353959ea5b2ea6..2dfc08a43f6fa8 100644 --- a/packages/interactivity/src/hooks.tsx +++ b/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 }` diff --git a/packages/interactivity/src/store.ts b/packages/interactivity/src/store.ts index 81905071702af2..87c9333c744296 100644 --- a/packages/interactivity/src/store.ts +++ b/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 ); } ); } diff --git a/packages/interactivity/src/utils.ts b/packages/interactivity/src/utils.ts index 4759a7b27a74a6..e201ff15a68927 100644 --- a/packages/interactivity/src/utils.ts +++ b/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 ); diff --git a/packages/interactivity/src/vdom.ts b/packages/interactivity/src/vdom.ts index 78f6d7032613a5..d5238cde49d8e7 100644 --- a/packages/interactivity/src/vdom.ts +++ b/packages/interactivity/src/vdom.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import { h } from 'preact'; +import { h, type ComponentChild, type JSX } from 'preact'; /** * Internal dependencies */ @@ -11,7 +11,7 @@ import { warn } from './utils/warn'; const ignoreAttr = `data-${ p }-ignore`; const islandAttr = `data-${ p }-interactive`; const fullPrefix = `data-${ p }-`; -const namespaces = []; +const namespaces: Array< string | null > = []; const currentNamespace = () => namespaces[ namespaces.length - 1 ] ?? null; // Regular expression for directive parsing. @@ -32,88 +32,109 @@ const directiveParser = new RegExp( // the reference, separated by `::`, like `some-namespace::state.somePath`. // Namespaces can contain any alphanumeric characters, hyphens, underscores or // forward slashes. References don't have any restrictions. -const nsPathRegExp = /^([\w-_\/]+)::(.+)$/; +const nsPathRegExp = /^(?[\w_\/-]+)::(?.+)$/; export const hydratedIslands = new WeakSet(); /** * Recursive function that transforms a DOM tree into vDOM. * - * @param {Node} root The root element or node to start traversing on. - * @return {import('preact').VNode[]} The resulting vDOM tree. + * @param root The root element or node to start traversing on. + * @return The resulting vDOM tree. */ -export function toVdom( root ) { +export function toVdom( root: Node ): Array< ComponentChild > { const treeWalker = document.createTreeWalker( root, - 205 // ELEMENT + TEXT + COMMENT + CDATA_SECTION + PROCESSING_INSTRUCTION + 205 // TEXT + CDATA_SECTION + COMMENT + PROCESSING_INSTRUCTION + ELEMENT ); - function walk( node ) { - const { attributes, nodeType, localName } = node; + function walk( + node: Node + ): [ ComponentChild ] | [ ComponentChild, Node | null ] { + const { nodeType } = node; + // TEXT_NODE (3) if ( nodeType === 3 ) { - return [ node.data ]; + return [ ( node as Text ).data ]; } + + // CDATA_SECTION_NODE (4) if ( nodeType === 4 ) { const next = treeWalker.nextSibling(); - node.replaceWith( new window.Text( node.nodeValue ) ); + ( node as CDATASection ).replaceWith( + new window.Text( ( node as CDATASection ).nodeValue ?? '' ) + ); return [ node.nodeValue, next ]; } + + // COMMENT_NODE (8) || PROCESSING_INSTRUCTION_NODE (7) if ( nodeType === 8 || nodeType === 7 ) { const next = treeWalker.nextSibling(); - node.remove(); + ( node as Comment | ProcessingInstruction ).remove(); return [ null, next ]; } + const elementNode = node as HTMLElement; + const { attributes } = elementNode; + const localName = elementNode.localName as keyof JSX.IntrinsicElements; + const props: Record< string, any > = {}; - const children = []; - const directives = []; + const children: Array< ComponentChild > = []; + const directives: Array< + [ name: string, namespace: string | null, value: unknown ] + > = []; let ignore = false; let island = false; for ( let i = 0; i < attributes.length; i++ ) { - const n = attributes[ i ].name; + const attributeName = attributes[ i ].name; if ( - n[ fullPrefix.length ] && - n.slice( 0, fullPrefix.length ) === fullPrefix + attributeName[ fullPrefix.length ] && + attributeName.slice( 0, fullPrefix.length ) === fullPrefix ) { - if ( n === ignoreAttr ) { + if ( attributeName === ignoreAttr ) { ignore = true; } else { - let [ ns, value ] = nsPathRegExp - .exec( attributes[ i ].value ) - ?.slice( 1 ) ?? [ null, attributes[ i ].value ]; + const regexCaptureGroups = nsPathRegExp.exec( + attributes[ i ].value + )?.groups; + const namespace = regexCaptureGroups?.namespace ?? null; + let value: any = + regexCaptureGroups?.value ?? attributes[ i ].value; try { - value = JSON.parse( value ); + value = value && JSON.parse( value ); } catch ( e ) {} - if ( n === islandAttr ) { + if ( attributeName === islandAttr ) { island = true; - namespaces.push( + const islandNamespace = + // eslint-disable-next-line no-nested-ternary typeof value === 'string' ? value - : value?.namespace ?? null - ); + : typeof value?.namespace === 'string' + ? value.namespace + : null; + namespaces.push( islandNamespace ); } else { - directives.push( [ n, ns, value ] ); + directives.push( [ attributeName, namespace, value ] ); } } - } else if ( n === 'ref' ) { + } else if ( attributeName === 'ref' ) { continue; } - props[ n ] = attributes[ i ].value; + props[ attributeName ] = attributes[ i ].value; } if ( ignore && ! island ) { return [ - h( localName, { + h< any, any >( localName, { ...props, - innerHTML: node.innerHTML, + innerHTML: elementNode.innerHTML, __directives: { ignore: true }, } ), ]; } if ( island ) { - hydratedIslands.add( node ); + hydratedIslands.add( elementNode ); } if ( directives.length ) { @@ -139,10 +160,11 @@ export function toVdom( root ) { ); } + // @ts-expect-error Fixed in upcoming preact release https://github.com/preactjs/preact/pull/4334 if ( localName === 'template' ) { - props.content = [ ...node.content.childNodes ].map( ( childNode ) => - toVdom( childNode ) - ); + props.content = [ + ...( elementNode as HTMLTemplateElement ).content.childNodes, + ].map( ( childNode ) => toVdom( childNode ) ); } else { let child = treeWalker.firstChild(); if ( child ) { diff --git a/packages/interactivity/tsconfig.json b/packages/interactivity/tsconfig.json index 0fb91956773f89..1d154e2089065d 100644 --- a/packages/interactivity/tsconfig.json +++ b/packages/interactivity/tsconfig.json @@ -4,8 +4,8 @@ "compilerOptions": { "rootDir": "src", "declarationDir": "build-types", - "checkJs": false, - "strict": false + + "noImplicitAny": false }, "include": [ "src/**/*" ] } diff --git a/tsconfig.base.json b/tsconfig.base.json index 1b30522ee52366..18388b7f71854c 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -7,7 +7,7 @@ "jsx": "preserve", "target": "esnext", "module": "esnext", - "lib": [ "dom", "esnext" ], + "lib": [ "DOM", "DOM.Iterable", "ESNext" ], "declaration": true, "declarationMap": true, "composite": true,