-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[embeddable rebuild] useBatchedOptionalPublishingSubjects #180221
Conversation
/ci |
/ci |
/ci |
/ci |
Pinging @elastic/kibana-presentation (Team:Presentation) |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea to separate out the optional from the required batcher. This way 90% of use cases can use the more efficient and smaller code. Nice jest tests also!
packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts
Outdated
Show resolved
Hide resolved
packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts
Show resolved
Hide resolved
packages/presentation/presentation_publishing/publishing_subject/publishing_batcher.ts
Show resolved
Hide resolved
@elasticmachine merge upstream |
…-cache --fix'" This reverts commit 3d67df3.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes made to image embeddable LGTM - makes sense now that useBatchedPublishingSubjects
needs all publishing subjects to be defined 👍
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes review only
…0221) Fixes elastic#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 <[email protected]>
…astic#180221)" This reverts commit 51c3fa5.
Fixes #180088
debounceTime(), skip(1)
toskip(1), debounceTime()
. Updates test to verify this change results in subscription getting fired when observable.next is called before debounceTime fires.useBatchedPublishingSubjects
touseBatchedOptionalPublishingSubjects
. RemoveuseMemo
since spreading subjects results in new array every time and useMemo does nothing.PresentationPanelInternal
to useuseBatchedOptionalPublishingSubjects
useBatchedPublishingSubjects
that types subjects asPublishingSubject[]
. New