Skip to content
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

[lexical-react] remove editor__DEPRECATED that has been deprecated for two years #6494

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

etrepum
Copy link
Collaborator

@etrepum etrepum commented Aug 6, 2024

Description

The editor__DEPRECATED prop has been deprecated for two years (#1407), this PR removes it.

Test plan

There were no tests for this feature, and its absence does not need new tests.

Copy link

vercel bot commented Aug 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 3:45am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 6, 2024 3:45am

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

size-limit report 📦

Path Size
lexical - cjs 29.02 KB (0%)
lexical - esm 28.85 KB (0%)
@lexical/rich-text - cjs 37.37 KB (0%)
@lexical/rich-text - esm 28.34 KB (0%)
@lexical/plain-text - cjs 36.03 KB (0%)
@lexical/plain-text - esm 25.6 KB (0%)
@lexical/react - cjs 39.27 KB (0%)
@lexical/react - esm 29.77 KB (+0.07% 🔺)

@@ -23,7 +23,6 @@ export type InitialEditorStateType =
| ((editor: LexicalEditor) => void);

export type InitialConfigType = $ReadOnly<{
editor__DEPRECATED?: LexicalEditor | null,
Copy link
Contributor

Choose a reason for hiding this comment

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

editor__DEPRECATED is used some places on www,

https://www.internalfb.com/code/search?q=repo%3Awww%20editor__DEPRECATED

im not sure how backwards compatible this would be

@zurfyx should we try cleaning up editor__DEPRECATED in www first before syncing this over?

Copy link
Member

Choose a reason for hiding this comment

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

@potatowagon Good point, we need to fix these occurrences first. I'll connect with you offline but unfortunately we should put this PR on hold for now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've converted to draft since we don't have a label for "on hold" or "don't merge"

@etrepum etrepum marked this pull request as draft August 6, 2024 15:27
@fantactuka
Copy link
Contributor

I just recalled Chriss @montlebalm was mentioning use case with reusing editor instance via this prop: #4986

Wonder if it's used by other folks too and (although it was marked as deprecated forever) it worth having explicit exception when it's passed

@zurfyx
Copy link
Member

zurfyx commented Aug 9, 2024

Wonder if it's used by other folks too and (although it was marked as deprecated forever) it worth having explicit exception when it's passed

@fantactuka FWIW we're also using the same workaround described in the issue internally and that's not going away anytime soon

@etrepum
Copy link
Collaborator Author

etrepum commented Aug 9, 2024

@fantactuka I can't think of a good reason to render a LexicalComposer when you already have the editor instance, LexicalComposerContext.Provider can be used instead

Copy link
Member

@zurfyx zurfyx left a comment

Choose a reason for hiding this comment

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

We can merge now! Thank you

@zurfyx zurfyx marked this pull request as ready for review August 13, 2024 13:39
@etrepum etrepum added this pull request to the merge queue Aug 13, 2024
Merged via the queue into facebook:main with commit 21e50ad Aug 13, 2024
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants