Skip to content

Commit

Permalink
Interactivity: Strict type checking (#59865)
Browse files Browse the repository at this point in the history
Enable `strictNullChecks` and improve types.

Co-authored-by: sirreal <[email protected]>
Co-authored-by: cbravobernal <[email protected]>
  • Loading branch information
3 people authored May 15, 2024
1 parent 7a9f1de commit 966f0f7
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 85 deletions.
5 changes: 4 additions & 1 deletion packages/interactivity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;

Expand All @@ -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 ];
Expand Down Expand Up @@ -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 ) ) &&
Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -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' )
Expand All @@ -217,7 +224,7 @@ const getGlobalEventDirective = ( type ) => {
const globalVar = type === 'window' ? window : document;
globalVar.addEventListener( eventName, cb );
return () => globalVar.removeEventListener( eventName, cb );
}, [] );
} );
} );
};
};
Expand Down Expand Up @@ -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 );
}
} );
} );
Expand Down Expand Up @@ -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;
}
} );
} );
Expand All @@ -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
Expand Down Expand Up @@ -462,6 +478,8 @@ export default () => {
type: Type,
props: { innerHTML, ...rest },
},
}: {
element: any;
} ) => {
// Preserve the initial inner HTML.
const cached = useMemo( () => innerHTML, [] );
Expand All @@ -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 =
Expand Down
24 changes: 16 additions & 8 deletions packages/interactivity/src/hooks.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/* @jsx createElement */

// eslint-disable-next-line eslint-comments/disable-enable-pair
/* eslint-disable react-hooks/exhaustive-deps */

/**
* External dependencies
*/
Expand All @@ -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';
Expand All @@ -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;
}
Expand All @@ -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.
*/
Expand All @@ -50,7 +58,7 @@ interface DirectiveArgs {
}

interface DirectiveCallback {
( args: DirectiveArgs ): VNode | void;
( args: DirectiveArgs ): VNode | null | void;
}

interface DirectiveOptions {
Expand All @@ -65,7 +73,7 @@ interface DirectiveOptions {

interface Scope {
evaluate: Evaluate;
context: Context< any >;
context: object;
ref: RefObject< HTMLElement >;
attributes: createElement.JSX.HTMLAttributes;
}
Expand Down Expand Up @@ -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'
Expand All @@ -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 ) );
}
Expand Down Expand Up @@ -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 }`
Expand Down
6 changes: 3 additions & 3 deletions packages/interactivity/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) ) {
Expand Down Expand Up @@ -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 );
} );
}
Expand Down
8 changes: 4 additions & 4 deletions packages/interactivity/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 );
Expand Down
Loading

0 comments on commit 966f0f7

Please sign in to comment.