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

Add default value for undefined case in markdown transformers #2453

Merged

Conversation

imnoahcook
Copy link
Contributor

Easy review

Context:
I was playing with the rich text playground and noticed that if you only used the ELEMENT_TRANFORMERS from @lexical/markdown, you will get a runtime error as it tries to iterate over the textFormat and textMatch fields, which are undefined in this case.

They're undefined because none of the element transformers have these fields in them.

I consider this a bug, because there shouldn't be any reason someone not to be able to use

for quote

  1. for list
    etc..
    without using the rest of the markdown exports

Solution:
Add a default empty array value to text-format and text-match

@facebook-github-bot
Copy link
Contributor

Hi @imnoahcook!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@vercel
Copy link

vercel bot commented Jun 17, 2022

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

Name Status Preview Updated
lexical ✅ Ready (Inspect) Visit Preview Jul 7, 2022 at 4:35AM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview Jul 7, 2022 at 4:35AM (UTC)

@imnoahcook
Copy link
Contributor Author

Screen Shot 2022-06-16 at 11 27 47 PM

Screenshot for context on what happens when you try to add `ELEMENT_TRANSFORMERS` to the `MarkdownShortcutPlugin` on the `rich-text-playground` example

@acywatson
Copy link
Contributor

Just needs the CLA signed, when you have a chance.

@imnoahcook
Copy link
Contributor Author

Signed CLA ✔️

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@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 Jun 21, 2022
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@acywatson
Copy link
Contributor

Ah, I forgot, we don't use nullish coalescing in Lexical for some reason that I can't quite recall at the moment. Something. about not wanting to set up the Babel plugin to transpile it, I think. You can use a logical OR, instead.

Copy link
Contributor

@fantactuka fantactuka left a comment

Choose a reason for hiding this comment

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

Minor update, looks good otherwise!

@acywatson
Copy link
Contributor

It looks like the playground vercel build is failing because of a missing script that was added recently. Rebasing might fix that.

@imnoahcook
Copy link
Contributor Author

Resolved changed by @fantactuka and rebased to pass tests.

@acywatson acywatson merged commit 09f2854 into facebook:main Jul 7, 2022
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.

4 participants