Skip to content

[Lens] Correctly use UserMessage longMessage as function #192492

Merged
markov00 merged 5 commits intoelastic:mainfrom
markov00:2024_09_10-fix_long_user_messages
Oct 14, 2024
Merged

[Lens] Correctly use UserMessage longMessage as function #192492
markov00 merged 5 commits intoelastic:mainfrom
markov00:2024_09_10-fix_long_user_messages

Conversation

@markov00
Copy link
Copy Markdown
Contributor

@markov00 markov00 commented Sep 10, 2024 •
edited by kibanamachine
Loading

Summary

After #167205 was merged, the UserMessage.longMessage was typed as longMessage: string | React.ReactNode | ((closePopover: () => void) => React.ReactNode);

With the upcoming React 18 upgrade, an error will become visible because ((closePopover: () => void) => React.ReactNode); can't be used as a ReactNode but it correctly needs to be called.

In this PR I've made the closePopover function being optional (to simplify the refactoring) and I've added the typecheck where needed.

@markov00 markov00 added release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v8.16.0 labels Sep 10, 2024
@markov00 markov00 force-pushed the 2024_09_10-fix_long_user_messages branch from 791eb08 to 3b3f7d5 Compare September 18, 2024 17:01
@markov00 markov00 force-pushed the 2024_09_10-fix_long_user_messages branch from 3b3f7d5 to 824f2f9 Compare September 18, 2024 17:10
@markov00 markov00 marked this pull request as ready for review September 26, 2024 09:34
@markov00 markov00 requested a review from a team as a code owner September 26, 2024 09:34
@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Sep 26, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.5MB 1.5MB +113.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

nickofthyme
nickofthyme approved these changes Sep 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM, I would just suggest extracting the duplicate logic.

Comment thread ...k/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_errors.tsx Outdated
@markov00 markov00 enabled auto-merge (squash) October 10, 2024 15:11
@markov00 markov00 merged commit e35507a into elastic:main Oct 14, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 14, 2024

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11332389816

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Oct 14, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 1465 1466 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lens 1.5MB 1.5MB +47.0B

History

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 14, 2024
@markov00
[Lens] Correctly use UserMessage longMessage as function (elastic#192492
ca2fa7b 
)

## Summary

After elastic#167205 was merged, the
`UserMessage.longMessage` was typed as `longMessage: string |
React.ReactNode | ((closePopover: () => void) => React.ReactNode);`

With the upcoming React 18 upgrade, an error will become visible because
`((closePopover: () => void) => React.ReactNode);` can't be used as a
ReactNode but it correctly needs to be called.

In this PR I've made the `closePopover` function being optional (to
simplify the refactoring) and I've added the typecheck where needed.

(cherry picked from commit e35507a)
@kibanamachine kibanamachine mentioned this pull request Oct 14, 2024
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Oct 14, 2024

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 14, 2024
@kibanamachine @markov00
[8.x] [Lens] Correctly use UserMessage longMessage as function (#192492…
7e8d54c 
…) (#196194)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] Correctly use UserMessage longMessage as function
(#192492)](#192492)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"marco.vettorello@elastic.co"},"sourceCommit":{"committedDate":"2024-10-14T17:34:36Z","message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Lens","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[Lens]
Correctly use UserMessage longMessage as function
","number":192492,"url":"https://github.com/elastic/kibana/pull/192492","mergeCommit":{"message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192492","number":192492,"mergeCommit":{"message":"[Lens]
Correctly use UserMessage longMessage as function (#192492)\n\n##
Summary\r\n\r\nAfter #167205 was
merged, the\r\n`UserMessage.longMessage` was typed as `longMessage:
string |\r\nReact.ReactNode | ((closePopover: () => void) =>
React.ReactNode);`\r\n\r\nWith the upcoming React 18 upgrade, an error
will become visible because\r\n`((closePopover: () => void) =>
React.ReactNode);` can't be used as a\r\nReactNode but it correctly
needs to be called.\r\n\r\nIn this PR I've made the `closePopover`
function being optional (to\r\nsimplify the refactoring) and I've added
the typecheck where
needed.","sha":"e35507a27d9c8df3fe5947c7227d6072d007dfa5"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees

No one assigned

Labels

Feature:Lens release_note:skip Skip the PR/issue when compiling release notes v8.16.0 v9.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@markov00 @kibana-ci @kibanamachine @elasticmachine @nickofthyme