Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interactivity: Strict type checking #59865

Merged
merged 24 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading