Properly convert successive shortcodes

[danielbachhuber]

Description

Removes memize() call around wp.shortcode.regexp() to avoid buggy behavior when converting successive shortcodes.

Fixes #7030

How has this been tested?

1. Add the following code to a Classic Editor post:

[foo one]

[foo two]

[foo three]

2. Open the post in Gutenberg.
3. Hit 'Convert to Blocks' and see that all shortcode instances have been converted to blocks.

Types of changes

Bug fix.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[danielbachhuber]

To debug the test case, add a console.log() to view beforeHTML:

When you do so, you'll notice that console.log() is only run three times, not four:

But why is segmentHTMLToShortcodeBlock() only being called three times, when it should be called four?

Adding another console.log() as the HTML is processed:

Take a look at the last string instance output:

The last instance is different than its peers in that: if you remove the shortcode, then the remaining HTML is simply a </p> tag (i.e. unbalanced HTML).

I don't have any good ideas on how to unwind this issue though.

@azaozz @iseulde @aduth Pointers?

[aduth]

Chatted about this with @danielbachhuber , seems to stem from this related Core issue:

https://core.trac.wordpress.org/ticket/35545

Which is that we're using shortcode regular expressions as stateful (RegExp#exec) while also memoizing their creation, thus causing unpredictable results like the one surfaced here.

There's not much to go by so far as the context for memoizing in the initial introduction of the shortcodes utility. Typically it'd be for performance, though in this case we're simply concatenating a string into a new regular expression, so doubtful we're really saving much (the overhead of memoization is likely more costly). Given that we are using the regular expressions as stateful, it may have been intentional to return a cached copy, but again the context is unclear.

My intuition is: Remove memize wrapper from wp.shortcode.regexp. This fixes #7030.

cc @iseulde @azaozz @koop

[aduth]

Looks good 👍

[aduth]

Minor: Simply for consistency, we could format this as the other plain functions in this file now:

export function regexp( tag ) {
	 // ...
}

[danielbachhuber]

Simply for consistency, we could format this as the other plain functions in this file now:

Good point, 1bf5ba0

[aduth]

Semi-colon should be removed.

[danielbachhuber]

I wouldn't hate you if you just removed it yourself :) a712bf7

[danielbachhuber]

Merging to incorporate into #7892. The failure is a codecov false positive.

[ktmn]

Does it also handle nesting?

[foo]
  [bar]
    [asd /]
  [/bar]
[/foo]
[foo]
  [bar][/bar]
[/foo]

Would it create 2 blocks?

[wpexplorer]

There seems to be issues still with converting blocks that are next to each other. I recorded a quick video to show you: https://a.cl.ly/jkuOjvOQ

There are many page builders out there that are shortcode based and basically everything added to the page are shortcodes and there isn't any space between them. So there is no way currently to convert these page builders to Blocks without having to first manually space out the shortcodes.

Thanks for all the great works you guys do over there!