[lexical-react] Annotate @deprecated to menuRenderFn with NodeContext… #8001
Conversation
…enuPlugin example link
…textMenuPlugin.tsx
thatmichael85
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon,
takuyakanbr and
zurfyx
as code owners
meta-cla
bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thatmichael85
requested review from
Copilot
and removed request for
acywatson,
etrepum,
fantactuka,
ivailop7,
potatowagon,
takuyakanbr and
zurfyx
There was a problem hiding this comment.
Pull Request Overview
This PR adds @deprecated annotations to the menuRenderFn property across multiple Lexical React plugins to guide users toward migrating to the newer LexicalNodeContextMenuPlugin API. The annotations include a link to an example implementation in the playground.
- Deprecation warnings added to
menuRenderFnproperties in TypeScript source files - Corresponding deprecation warnings added to Flow type definition files
- Enhanced deprecation documentation for the
LexicalContextMenuPlugincomponent itself
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/lexical-react/src/LexicalTypeaheadMenuPlugin.tsx | Added deprecation JSDoc to menuRenderFn property |
| packages/lexical-react/src/LexicalNodeMenuPlugin.tsx | Added deprecation JSDoc to menuRenderFn property |
| packages/lexical-react/src/LexicalContextMenuPlugin.tsx | Added deprecation JSDoc to menuRenderFn property and enhanced function-level deprecation notice |
| packages/lexical-react/src/LexicalAutoEmbedPlugin.tsx | Added deprecation JSDoc to menuRenderFn property |
| packages/lexical-react/flow/LexicalTypeaheadMenuPlugin.js.flow | Added deprecation JSDoc to Flow type definition |
| packages/lexical-react/flow/LexicalNodeMenuPlugin.js.flow | Added deprecation JSDoc to Flow type definition |
| packages/lexical-react/flow/LexicalAutoEmbedPlugin.js.flow | Added deprecation JSDoc to Flow type definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
| * @deprecated Use LexicalNodeContextMenuPlugin instead. Here is an example for using NodeContextMenuPlugin: | ||
| * https://github.com/facebook/lexical/blob/main/packages/lexical-playground/src/plugins/ContextMenuPlugin/index.tsx | ||
| */ | ||
| menuRenderFn: ContextMenuRenderFn<TOption>; |
There was a problem hiding this comment.
Besides marking it as deprecated, is it possible to restore the new API that @ivailop7 introduced so that we only get to this as a workaround for surfaces that are time-consuming to migrate? I see there's still a few playground callsites that are using this property already.
ivailop7
left a comment
ivailop7
left a comment
There was a problem hiding this comment.
A lot of incorrect deprecation comments in this PR.
Putting this here to clarify:
LexicalNodeContextMenu is a replacement ONLY for LexicalContextMenu. The Playground example has been using the new one for 5-6 months+. So the LexicalContextMenu should be fully deletable in its current state.
No other deprecation of menuRenderFn in AutoEmbed, NodeMenu or Typeahead is happening, since they need to have their interface changed, to use the one I did in the now reverted PR, but maybe keep menuRenderFn alongside it for now, as an escape hatch.
| dismissFn: () => void, | ||
| ) => Array<AutoEmbedOption>, | ||
| /** | ||
| * @deprecated Use NodeContextMenuPlugin instead. Here is an example for using NodeContextMenuPlugin: |
There was a problem hiding this comment.
This is not correct. NodeContextMenuPlugin is not a replacement for AutoEmbedPlugin
| onClose?: () => void, | ||
| onOpen?: (resolution: MenuResolution) => void, | ||
| /** | ||
| * @deprecated Use NodeContextMenuPlugin instead. Here is an example for using NodeContextMenuPlugin: |
There was a problem hiding this comment.
This is also not correct. NodeContextMenuPlugin is a replacement only for ContextMenuPlugin, nothing else.
| matchingString: string, | ||
| ) => void, | ||
| options: Array<TOption>, | ||
| /** |
There was a problem hiding this comment.
Remove this one as well please.
| embedFn: () => void, | ||
| dismissFn: () => void, | ||
| ) => Array<AutoEmbedOption>; | ||
| /** |
| nodeKey: NodeKey | null; | ||
| onClose?: () => void; | ||
| onOpen?: (resolution: MenuResolution) => void; | ||
| /** |
| matchingString: string, | ||
| ) => void; | ||
| options: Array<TOption>; | ||
| /** |
There was a problem hiding this comment.
Please remove this as well.
What
Annotate menuRenderFn with @deprecated and point to the link in the playground
Why
To alert users that this prop is deprecated and to migrate to LexicalNodeContextMenu apis.
The annotation at packages/lexical-react/flow/LexicalContextMenuPlugin.js.flow isn't too helpful
e.g
