From 83cb0e440e4082927d231b2b2488ef432cbcaf74 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Wed, 10 Apr 2024 07:27:59 -0600 Subject: [PATCH] [embeddable rebuild] useBatchedOptionalPublishingSubjects (#180221) Fixes https://github.com/elastic/kibana/issues/180088 1. Changes observable pipe from `debounceTime(), skip(1)` to `skip(1), debounceTime()`. Updates test to verify this change results in subscription getting fired when observable.next is called before debounceTime fires. 2. rename `useBatchedPublishingSubjects` to `useBatchedOptionalPublishingSubjects`. Remove `useMemo` since spreading subjects results in new array every time and useMemo does nothing. 3. Update `PresentationPanelInternal` to use `useBatchedOptionalPublishingSubjects` 4. create new `useBatchedPublishingSubjects` that types subjects as `PublishingSubject[]`. New --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../presentation_publishing/index.ts | 1 + .../publishing_subject/index.ts | 5 +- .../publishing_subject/publishing_batcher.ts | 59 +++++++++++-- .../publishing_subject.test.tsx | 88 +++++++++++++------ .../public/components/image_embeddable.tsx | 3 +- .../presentation_panel_context_menu.tsx | 7 +- .../presentation_panel_internal.tsx | 4 +- 7 files changed, 128 insertions(+), 39 deletions(-) diff --git a/packages/presentation/presentation_publishing/index.ts b/packages/presentation/presentation_publishing/index.ts index d3764da74c6d2..e4476cbd2cff9 100644 --- a/packages/presentation/presentation_publishing/index.ts +++ b/packages/presentation/presentation_publishing/index.ts @@ -111,6 +111,7 @@ export { } from './interfaces/titles/publishes_panel_title'; export { initializeTitles, type SerializedTitles } from './interfaces/titles/titles_api'; export { + useBatchedOptionalPublishingSubjects, useBatchedPublishingSubjects, usePublishingSubject, useStateFromPublishingSubject, diff --git a/packages/presentation/presentation_publishing/publishing_subject/index.ts b/packages/presentation/presentation_publishing/publishing_subject/index.ts index 5dbd2eb95579a..022c4170f6cde 100644 --- a/packages/presentation/presentation_publishing/publishing_subject/index.ts +++ b/packages/presentation/presentation_publishing/publishing_subject/index.ts @@ -6,7 +6,10 @@ * Side Public License, v 1. */ -export { useBatchedPublishingSubjects } from './publishing_batcher'; +export { + useBatchedOptionalPublishingSubjects, + useBatchedPublishingSubjects, +} from './publishing_batcher'; export { useStateFromPublishingSubject, usePublishingSubject } from './publishing_subject'; export type { PublishingSubject, diff --git a/packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts b/packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts index 4624e43c2a0d1..f04661573d918 100644 --- a/packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts +++ b/packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { combineLatest, debounceTime, skip } from 'rxjs'; import { AnyPublishingSubject, PublishingSubject, UnwrapPublishingSubjectTuple } from './types'; @@ -25,25 +25,28 @@ const hasSubjectsArrayChanged = ( /** * Batches the latest values of multiple publishing subjects into a single object. Use this to avoid unnecessary re-renders. - * You should avoid using this hook with subjects that your component pushes values to on user interaction, as it can cause a slight delay. + * Use when `subjects` may not be defined on initial component render. + * * @param subjects Publishing subjects array. * When 'subjects' is expected to change, 'subjects' must be part of component react state. */ -export const useBatchedPublishingSubjects = ( +export const useBatchedOptionalPublishingSubjects = < + SubjectsType extends [...AnyPublishingSubject[]] +>( ...subjects: [...SubjectsType] ): UnwrapPublishingSubjectTuple => { const isFirstRender = useRef(true); - /** - * memoize and deep diff subjects to avoid rebuilding the subscription when the subjects are the same. - */ + const previousSubjects = useRef(subjects); - const subjectsToUse = useMemo(() => { + // Can not use 'useMemo' because 'subjects' gets a new reference on each call because of spread + const subjectsToUse = (() => { + // avoid rebuilding the subscription when the subjects are the same if (!hasSubjectsArrayChanged(previousSubjects.current ?? [], subjects)) { return previousSubjects.current; } previousSubjects.current = subjects; return subjects; - }, [subjects]); + })(); /** * Set up latest published values state, initialized with the current values of the subjects. @@ -94,6 +97,46 @@ export const useBatchedPublishingSubjects = >] +>( + ...subjects: [...SubjectsType] +): UnwrapPublishingSubjectTuple => { + /** + * Set up latest published values state, initialized with the current values of the subjects. + */ + const [latestPublishedValues, setLatestPublishedValues] = useState< + UnwrapPublishingSubjectTuple + >(() => unwrapPublishingSubjectArray(subjects)); + + /** + * Subscribe to all subjects and update the latest values when any of them change. + */ + useEffect(() => { + const subscription = combineLatest(subjects) + .pipe( + // When a new observer subscribes to a BehaviorSubject, it immediately receives the current value. Skip this emit. + skip(1), + debounceTime(0) + ) + .subscribe((values) => { + setLatestPublishedValues(values as UnwrapPublishingSubjectTuple); + }); + return () => subscription.unsubscribe(); + // 'subjects' gets a new reference on each call because of spread + // Use 'useBatchedOptionalPublishingSubjects' when 'subjects' are expected to change. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + return latestPublishedValues; +}; + const unwrapPublishingSubjectArray = ( subjects: T ): UnwrapPublishingSubjectTuple => { diff --git a/packages/presentation/presentation_publishing/publishing_subject/publishing_subject.test.tsx b/packages/presentation/presentation_publishing/publishing_subject/publishing_subject.test.tsx index e58ca06d54f9b..ec0d80c0dd3c9 100644 --- a/packages/presentation/presentation_publishing/publishing_subject/publishing_subject.test.tsx +++ b/packages/presentation/presentation_publishing/publishing_subject/publishing_subject.test.tsx @@ -11,11 +11,14 @@ import { BehaviorSubject } from 'rxjs'; import { render, screen, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom'; import userEvent from '@testing-library/user-event'; -import { useBatchedPublishingSubjects } from './publishing_batcher'; +import { + useBatchedPublishingSubjects, + useBatchedOptionalPublishingSubjects, +} from './publishing_batcher'; import { useStateFromPublishingSubject } from './publishing_subject'; import { PublishingSubject } from './types'; -describe('useBatchedPublishingSubjects', () => { +describe('publishing subject', () => { describe('render', () => { let subject1: BehaviorSubject; let subject2: BehaviorSubject; @@ -56,7 +59,6 @@ describe('useBatchedPublishingSubjects', () => { <>