[lexical-markdown] Bug fix: Prevent transform from removing nodes if the replace function returns false

[rodrigomotion]

Description

Currently if you return false from a Markdown transformer replace function, the expectation is that the transform will be cancelled, however the matched nodes are still being modified (removed) because the leading node gets removed before checking the return value of the replace function. This prevents implementing some functionality, such as not turning headers into numbered lists, and could cause other issues since when the next transform runs it won't have the leading node anymore.

Describe the changes in this pull request

I tried to keep it simple and just moved the remove function so that it runs only if the replace function does not return false. All tests pass.

Closes #7563

Test plan

Ran all the unit and e2e tests and they all pass.

Before and after video

https://share.cleanshot.com/JYWVlb4d

[vercel]

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	May 23, 2025 8:44pm
lexical-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 23, 2025 8:44pm

[rodrigomotion]

Seems like one of the test suites is timing out but it's unrelated to this change?

[etrepum]

Likely, I will restart it. Do you plan to write some additional tests or make any other changes before review? It is still marked WIP.

[rodrigomotion]

Likely, I will restart it. Do you plan to write some additional tests or make any other changes before review? It is still marked WIP.

I was trying to write a test, but I'm not sure how to set this up without changing some of the transformers, so I won't unless someone thinks it's required and can guide me on how to set a up a test for this. If the current tests pass I'll leave it as is.

[etrepum]

You should be able to write a unit test which wouldn't require changing other code, there's at least one transformer defined in the unit tests for the markdown package to test something that can't be reproduced in the playground.

[rodrigomotion]

You should be able to write a unit test which wouldn't require changing other code, there's at least one transformer defined in the unit tests for the markdown package to test something that can't be reproduced in the playground.

Ok I added a couple of tests, the first of which fails for the old behaviour. I added one for the normal header transforms to also avoid regressions with that and confirm my test was actually testing the right thing.

[etrepum]

Tentatively looks good, other than this typo in the tests. Will wait to see the result of the full test run.

[etrepum]

Suggested change

	export const CENCELED_HEADING_REPLACE_EXAMPLE: ElementTransformer = {
	export const CANCELED_HEADING_REPLACE_EXAMPLE: ElementTransformer = {

[rodrigomotion]

whoops, will fix that, thanks!

[etrepum]

Suggested change

	registerMarkdownShortcuts(editor, [CENCELED_HEADING_REPLACE_EXAMPLE]);
	registerMarkdownShortcuts(editor, [CANCELED_HEADING_REPLACE_EXAMPLE]);