From 655b08c8659adce2854447ac92dbc34455afbc95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dami=C3=A1n=20Su=C3=A1rez?= Date: Thu, 19 Oct 2023 18:54:39 +0100 Subject: [PATCH] AI Extension: improve performance bug when extending blocks with AI Assistant (#33681) * do not useTextContentFromSelectedBlocks() hook * changelog * do not import useTextContentFromSelectedBlocks() hook * create getBlocksContent() helper * move replaceWithAiAssistantBlock() to AI Dropdown * move the AI assistant logic to the dropdown cmp * remove unused component * use useSelect() inestad of select() * set options arg optional * remove unneeded excluding process * move getBlocksContent() to dropdown cmp * remove uneeded usage of useCallback * clean * remove requestingState prop * remove exclude usage * avoid using internal getBlockTextContent helper fn * dont pass down disabled prop to dropdown instance * remove useCallback usage * remove useTextContentFromSelectedBlocks() def * set ExtendedBlockProp as type explicitely * remove obsolte block props * clean types * discard getBlockTextContent() * add a CAUTION message about performance risk --- ...-perfomance-issue-extending-blocks-with-ai | 4 + .../ai-assistant-controls/index.tsx | 216 ++++++++++-------- .../ai-assistant/with-ai-assistant.tsx | 116 +--------- .../index.ts | 33 --- .../ai-assistant/lib/utils/block-content.ts | 27 +-- 5 files changed, 134 insertions(+), 262 deletions(-) create mode 100644 projects/plugins/jetpack/changelog/update-fix-perfomance-issue-extending-blocks-with-ai delete mode 100644 projects/plugins/jetpack/extensions/blocks/ai-assistant/hooks/use-text-content-from-selected-blocks/index.ts diff --git a/projects/plugins/jetpack/changelog/update-fix-perfomance-issue-extending-blocks-with-ai b/projects/plugins/jetpack/changelog/update-fix-perfomance-issue-extending-blocks-with-ai new file mode 100644 index 0000000000000..8f1fbf424d86e --- /dev/null +++ b/projects/plugins/jetpack/changelog/update-fix-perfomance-issue-extending-blocks-with-ai @@ -0,0 +1,4 @@ +Significance: patch +Type: bugfix + +AI Extension: improve performance bug when extending blocks with AI Assistant diff --git a/projects/plugins/jetpack/extensions/blocks/ai-assistant/components/ai-assistant-controls/index.tsx b/projects/plugins/jetpack/extensions/blocks/ai-assistant/components/ai-assistant-controls/index.tsx index 40d4d08c09025..1247f03e1bb11 100644 --- a/projects/plugins/jetpack/extensions/blocks/ai-assistant/components/ai-assistant-controls/index.tsx +++ b/projects/plugins/jetpack/extensions/blocks/ai-assistant/components/ai-assistant-controls/index.tsx @@ -2,20 +2,17 @@ * External dependencies */ import { aiAssistantIcon } from '@automattic/jetpack-ai-client'; -import { - MenuItem, - MenuGroup, - CustomSelectControl, - ToolbarButton, - Dropdown, -} from '@wordpress/components'; +import { useAnalytics } from '@automattic/jetpack-shared-extension-utils'; +import { getBlockContent } from '@wordpress/blocks'; +import { MenuItem, MenuGroup, ToolbarButton, Dropdown } from '@wordpress/components'; +import { useSelect, useDispatch } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; import { post, postContent, postExcerpt, termDescription } from '@wordpress/icons'; -import classNames from 'classnames'; import React from 'react'; /** * Internal dependencies */ +import { getStoreBlockId } from '../../extensions/ai-assistant/with-ai-assistant'; import { PROMPT_TYPE_CHANGE_TONE, PROMPT_TYPE_CORRECT_SPELLING, @@ -24,12 +21,14 @@ import { PROMPT_TYPE_SUMMARIZE, PROMPT_TYPE_CHANGE_LANGUAGE, } from '../../lib/prompt'; +import { transformToAIAssistantBlock } from '../../transforms'; import { I18nMenuDropdown } from '../i18n-dropdown-control'; import { ToneDropdownMenu } from '../tone-dropdown-control'; import './style.scss'; /** * Types and constants */ +import type { ExtendedBlockProp } from '../../extensions/ai-assistant'; import type { PromptTypeProp } from '../../lib/prompt'; import type { ToneProp } from '../tone-dropdown-control'; @@ -48,15 +47,6 @@ const QUICK_EDIT_KEY_MAKE_LONGER = 'make-longer' as const; // Ask AI Assistant option export const KEY_ASK_AI_ASSISTANT = 'ask-ai-assistant' as const; -const QUICK_EDIT_KEY_LIST = [ - QUICK_EDIT_KEY_CORRECT_SPELLING, - QUICK_EDIT_KEY_SIMPLIFY, - QUICK_EDIT_KEY_SUMMARIZE, - QUICK_EDIT_KEY_MAKE_LONGER, -] as const; - -type AiAssistantKeyProp = ( typeof QUICK_EDIT_KEY_LIST )[ number ] | typeof KEY_ASK_AI_ASSISTANT; - const quickActionsList = [ { name: __( 'Correct spelling and grammar', 'jetpack' ), @@ -91,49 +81,117 @@ export type AiAssistantDropdownOnChangeOptionsArgProps = { type AiAssistantControlComponentProps = { /* - * Can be used to externally control the value of the control. Optional. + * The block type. Required. */ - key?: AiAssistantKeyProp | string; + blockType: ExtendedBlockProp; +}; - /* - * The label to use for the dropdown. Optional. - */ - label?: string; +/** + * Given a list of blocks, it returns their content as a string. + * @param {Array} blocks - The list of blocks. + * @returns {string} The content of the blocks as a string. + */ +export function getBlocksContent( blocks ) { + return blocks + .filter( block => block != null ) // Safeguard against null or undefined blocks + .map( block => getBlockContent( block ) ) + .join( '\n\n' ); +} - /* - * A list of quick edits to exclude from the dropdown. - */ - exclude?: AiAssistantKeyProp[]; +export default function AiAssistantDropdown( { blockType }: AiAssistantControlComponentProps ) { + const toolbarLabel = __( 'AI Assistant', 'jetpack' ); /* - * Whether the dropdown is requesting suggestions from AI. + * Let's disable the eslint rule for this line. + * @todo: fix by using StoreDescriptor, or something similar */ - requestingState?: string; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const { getSelectedBlockClientIds, getBlocksByClientId } = useSelect( 'core/block-editor' ); + const { removeBlocks, replaceBlock } = useDispatch( 'core/block-editor' ); - /* - * Whether the dropdown is disabled. - */ - disabled?: boolean; + const { tracks } = useAnalytics(); - onChange: ( item: PromptTypeProp, options?: AiAssistantDropdownOnChangeOptionsArgProps ) => void; + const requestSuggestion = ( + promptType: PromptTypeProp, + options: AiAssistantDropdownOnChangeOptionsArgProps + ) => { + const clientIds = getSelectedBlockClientIds(); + const blocks = getBlocksByClientId( clientIds ); + const content = getBlocksContent( blocks ); - onReplace: () => void; -}; + const [ firstBlock ] = blocks; + const [ firstClientId, ...otherBlocksIds ] = clientIds; -export default function AiAssistantDropdown( { - key, - label, - exclude = [], - requestingState, - disabled, - onChange, - onReplace, -}: AiAssistantControlComponentProps ) { - const quickActionsListFiltered = quickActionsList.filter( - quickAction => ! exclude.includes( quickAction.key ) - ); - const toolbarLabel = - requestingState === 'suggesting' ? null : label || __( 'AI Assistant', 'jetpack' ); + const extendedBlockAttributes = { + ...( firstBlock?.attributes || {} ), // firstBlock.attributes should never be undefined, but still add a fallback + content, + }; + + const newAIAssistantBlock = transformToAIAssistantBlock( blockType, extendedBlockAttributes ); + + /* + * Store in the local storage the client id + * of the block that need to auto-trigger the AI Assistant request. + * @todo: find a better way to update the content, + * probably using a new store triggering an action. + */ + + // Storage client Id, prompt type, and options. + const storeObject = { + clientId: firstClientId, + type: promptType, + options: { ...options, contentType: 'generated', fromExtension: true }, // When converted, the original content must be treated as generated + }; + + localStorage.setItem( + getStoreBlockId( newAIAssistantBlock.clientId ), + JSON.stringify( storeObject ) + ); + + /* + * Replace the first block with the new AI Assistant block instance. + * This block contains the original content, + * even for multiple blocks selection. + */ + replaceBlock( firstClientId, newAIAssistantBlock ); + + // It removes the rest of the blocks in case there are more than one. + removeBlocks( otherBlocksIds ); + }; + + const onChange = ( + promptType: PromptTypeProp, + options: AiAssistantDropdownOnChangeOptionsArgProps = {} + ) => { + tracks.recordEvent( 'jetpack_editor_ai_assistant_extension_toolbar_button_click', { + suggestion: promptType, + block_type: blockType, + } ); + + requestSuggestion( promptType, options ); + }; + + const replaceWithAiAssistantBlock = () => { + const clientIds = getSelectedBlockClientIds(); + const blocks = getBlocksByClientId( clientIds ); + const content = getBlocksContent( blocks ); + + const [ firstClientId, ...otherBlocksIds ] = clientIds; + const [ firstBlock ] = blocks; + + const extendedBlockAttributes = { + ...( firstBlock?.attributes || {} ), // firstBlock.attributes should never be undefined, but still add a fallback + content, + }; + + replaceBlock( + firstClientId, + transformToAIAssistantBlock( blockType, extendedBlockAttributes ) + ); + + removeBlocks( otherBlocksIds ); + }; return ( { return ( ); } } renderContent={ ( { onClose: closeDropdown } ) => ( - - { ! exclude.includes( KEY_ASK_AI_ASSISTANT ) && ( - -
- { __( 'Ask AI Assistant', 'jetpack' ) } -
- - ) } + + +
+ { __( 'Ask AI Assistant', 'jetpack' ) } +
+ - { quickActionsListFiltered.map( quickAction => ( + { quickActionsList.map( quickAction => (
{ quickAction.name }
 @@ -205,27 +257,3 @@ export default function AiAssistantDropdown( { /> ); } - -export function QuickEditsSelectControl( { - key, - label, - exclude = [], - onChange, -}: AiAssistantControlComponentProps ) { - // Initial value. If not found, use empty. - const value = quickActionsList.find( quickAction => quickAction.key === key ) || ''; - - // Exclude when required. - const quickActionsListFiltered = exclude.length - ? quickActionsList.filter( quickAction => ! exclude.includes( quickAction.key ) ) - : quickActionsList; - - return ( - onChange( selectedItem ) } - /> - ); -} diff --git a/projects/plugins/jetpack/extensions/blocks/ai-assistant/extensions/ai-assistant/with-ai-assistant.tsx b/projects/plugins/jetpack/extensions/blocks/ai-assistant/extensions/ai-assistant/with-ai-assistant.tsx index bce8870d9d3f1..6585397a611f7 100644 --- a/projects/plugins/jetpack/extensions/blocks/ai-assistant/extensions/ai-assistant/with-ai-assistant.tsx +++ b/projects/plugins/jetpack/extensions/blocks/ai-assistant/extensions/ai-assistant/with-ai-assistant.tsx @@ -1,26 +1,13 @@ /** * External dependencies */ -import { useAnalytics } from '@automattic/jetpack-shared-extension-utils'; import { BlockControls } from '@wordpress/block-editor'; import { createHigherOrderComponent } from '@wordpress/compose'; -import { useDispatch } from '@wordpress/data'; -import { useCallback } from '@wordpress/element'; import React from 'react'; /** * Internal dependencies */ -import AiAssistantDropdown, { - AiAssistantDropdownOnChangeOptionsArgProps, - KEY_ASK_AI_ASSISTANT, -} from '../../components/ai-assistant-controls'; -import useTextContentFromSelectedBlocks from '../../hooks/use-text-content-from-selected-blocks'; -import { getRawTextFromHTML } from '../../lib/utils/block-content'; -import { transformToAIAssistantBlock } from '../../transforms'; -/* - * Types - */ -import type { PromptTypeProp } from '../../lib/prompt'; +import AiAssistantDropdown from '../../components/ai-assistant-controls'; export function getStoreBlockId( clientId ) { return `ai-assistant-block-${ clientId }`; @@ -33,111 +20,22 @@ export function getStoreBlockId( clientId ) { export const withAIAssistant = createHigherOrderComponent( BlockEdit => props => { const { name: blockType } = props; - const { removeBlocks, replaceBlock } = useDispatch( 'core/block-editor' ); - const { content, clientIds, blocks } = useTextContentFromSelectedBlocks(); - const { tracks } = useAnalytics(); - - /* - * Set exclude dropdown options. - * - Exclude "Ask AI Assistant" for core/list-item block. - */ - const exclude = []; - if ( blockType === 'core/list-item' ) { - exclude.push( KEY_ASK_AI_ASSISTANT ); - } - - const requestSuggestion = useCallback( - ( promptType: PromptTypeProp, options: AiAssistantDropdownOnChangeOptionsArgProps ) => { - const [ firstBlock ] = blocks; - const [ firstClientId, ...otherBlocksIds ] = clientIds; - - const extendedBlockAttributes = { - ...( firstBlock?.attributes || {} ), // firstBlock.attributes should never be undefined, but still add a fallback - content, - }; - - const newAIAssistantBlock = transformToAIAssistantBlock( - blockType, - extendedBlockAttributes - ); - - /* - * Store in the local storage the client id - * of the block that need to auto-trigger the AI Assistant request. - * @todo: find a better way to update the content, - * probably using a new store triggering an action. - */ - - // Storage client Id, prompt type, and options. - const storeObject = { - clientId: firstClientId, - type: promptType, - options: { ...options, contentType: 'generated', fromExtension: true }, // When converted, the original content must be treated as generated - }; - - localStorage.setItem( - getStoreBlockId( newAIAssistantBlock.clientId ), - JSON.stringify( storeObject ) - ); - - /* - * Replace the first block with the new AI Assistant block instance. - * This block contains the original content, - * even for multiple blocks selection. - */ - replaceBlock( firstClientId, newAIAssistantBlock ); - - // It removes the rest of the blocks in case there are more than one. - removeBlocks( otherBlocksIds ); - }, - [ blocks, clientIds, content, blockType, replaceBlock, removeBlocks ] - ); - - const handleChange = useCallback( - ( promptType: PromptTypeProp, options: AiAssistantDropdownOnChangeOptionsArgProps ) => { - tracks.recordEvent( 'jetpack_editor_ai_assistant_extension_toolbar_button_click', { - suggestion: promptType, - block_type: blockType, - } ); - - requestSuggestion( promptType, options ); - }, - [ tracks, requestSuggestion, blockType ] - ); - - const replaceWithAiAssistantBlock = useCallback( () => { - const [ firstClientId, ...otherBlocksIds ] = clientIds; - const [ firstBlock ] = blocks; - - const extendedBlockAttributes = { - ...( firstBlock?.attributes || {} ), // firstBlock.attributes should never be undefined, but still add a fallback - content, - }; - - replaceBlock( - firstClientId, - transformToAIAssistantBlock( blockType, extendedBlockAttributes ) - ); - - removeBlocks( otherBlocksIds ); - }, [ blocks, blockType, content, replaceBlock, clientIds, removeBlocks ] ); const blockControlProps = { group: 'block', }; - const rawContent = getRawTextFromHTML( content ); + /* + * CAUTION: code added before this line will be executed for all extended blocks, + * defined by the EXTENDED_BLOCKS constant in ../, not just the selected blocks. + * Code added above this line should be carefully evaluated for its impact on performance. + */ return ( <> - + ); diff --git a/projects/plugins/jetpack/extensions/blocks/ai-assistant/hooks/use-text-content-from-selected-blocks/index.ts b/projects/plugins/jetpack/extensions/blocks/ai-assistant/hooks/use-text-content-from-selected-blocks/index.ts deleted file mode 100644 index c367e72fc5635..0000000000000 --- a/projects/plugins/jetpack/extensions/blocks/ai-assistant/hooks/use-text-content-from-selected-blocks/index.ts +++ /dev/null @@ -1,33 +0,0 @@ -/** - * Internal dependencies - */ -import { getBlockTextContent } from '../../lib/utils/block-content'; -import useSelectedBlocks from '../use-selected-blocks'; -/* - * Types - */ -import type { GetSelectedBlocksProps } from '../use-selected-blocks'; - -type GetTextContentFromBlocksProps = GetSelectedBlocksProps & { - content: string; -}; - -/** - * Returns the text content from all selected blocks. - * - * @returns {GetTextContentFromBlocksProps} The text content. - */ - -export default function useTextContentFromSelectedBlocks(): GetTextContentFromBlocksProps { - const selected = useSelectedBlocks(); - - return { - ...selected, - content: selected.blocks - ? selected.blocks - .filter( block => block != null ) // Safeguard against null or undefined blocks - .map( block => getBlockTextContent( block.clientId ) ) - .join( '\n\n' ) - : '', - }; -} diff --git a/projects/plugins/jetpack/extensions/blocks/ai-assistant/lib/utils/block-content.ts b/projects/plugins/jetpack/extensions/blocks/ai-assistant/lib/utils/block-content.ts index b09db572e1692..5a06d8964128e 100644 --- a/projects/plugins/jetpack/extensions/blocks/ai-assistant/lib/utils/block-content.ts +++ b/projects/plugins/jetpack/extensions/blocks/ai-assistant/lib/utils/block-content.ts @@ -57,35 +57,10 @@ export function getTextContentFromInnerBlocks( clientId: string ) { return block.innerBlocks .filter( blq => blq != null ) // Safeguard against null or undefined blocks - .map( blq => getBlockTextContent( blq.clientId ) ) + .map( blq => getBlockContent( blq.clientId ) ) .join( '\n\n' ); } -/** - * Return the block content from the given block clientId using the `getBlockContent` function. - * - * @param {string} clientId - The block clientId. - * @returns {string} The block content. - */ -export function getBlockTextContent( clientId: string ): string { - if ( ! clientId ) { - return ''; - } - - const editor = select( 'core/block-editor' ); - const block = editor.getBlock( clientId ); - - /* - * In some context, the block can be undefined, - * for instance, when previewing the block. - */ - if ( ! block ) { - return ''; - } - - return getBlockContent( block ); -} - /** * Extract raw text from HTML content *