Large placeholders are not processed

Hi @dinazaur, you did a really nice job explaining the issue! :)

One more idea though - in approach 1 I don't like an idea of processing chunks out of order; probably we could also check if there are previos unprocessed chunks each time a _new_ chunk is loaded?

Thank you for reporting.

Can we get a test only patch showing the issue.

Could tests not be added like BigPipeStrategyTest

Ah okay I’ll leave for another reviewer then to see what they think. Don’t want to mark anything without test coverage personally.

To save space what do you think of doing

<div id="big-pipe-large-content">
  {% for i in 0..1000000 %}
    boing
  {% endfor %}
</div>

In the template.

Also hiding the test-only patch so the review bot doesn't pick it up.

+1 RTBC

I can confirm the issue since Drupal 10.0. I'm using an XYBlockBase class inheriting BlockBase in my module, that lazy-loads the block body using the big-pipe technology. This class is extensively used in my module, mainly to load lists of rendered entities with content sizes from 10k to more than 1M. Since I've updated my Drupal instance to version 10 last week, all these blocks didn't load its content anymore due to the above described issue of an incomplete script-node, when the MutationObserver is triggered.

Side note: The upgrade to Drupal 10 worked on my local development environment for smaller content (<40k), but failed on the stage server. So the streamed size of the script-node content, when the MutationsObserver is triggered, depends on the runtime environment.

The patch #2 solves this issue for all lazy-loading blocks. The solution of patch #2 is clean and not invasive, because all non-JSON-parsable script node processing is just postponed until the document loading is complete (DOMContentLoaded event).

PS: The references to the non-parsable script nodes stay the same, because only the script-node content is updated by the HTTP streaming of large placeholders.

Bumping this to critical - bits of the page just randomly go missing rendering the site potentially unusable, multiple reports in what look like duplicate issues.

+1 on the approach, could use a couple of comments for future us.

Needs work to address @nod_'s feedback.

Hiding patches to make it easier to show work is in the MR.

Appears the threads and feedback have been addressed.

I would really like to see some manual testing on this issue from the people reporting #3390178: big_pipe sometimes fails to load blocks (or with steps to reproduce from that issue), which seems to be more about lots of placeholders rather than large placeholders but feels like could be a different symptom of the same thing.

If it doesn't fix the issues on there, we might want to consider rolling back #3196973: Use Mutation observer for BigPipe replacements altogether than opening a new issue to re-introduce it along with this fix and anything else necessary.

On the other hand if it does fix people's issues on there, I'd be a lot more confident about committing it.

https://git.drupalcode.org/project/drupal/-/merge_requests/4778, which is the MR version of #2 (with additional comments and tests), fixes the issue in my case, where I was getting a big block not being rendered through BigPipe.

I believe that all the feedback was addressed or explained. The tests come up green for DrupalCI and for GitLab CI.

I think it needs a rebase. Pending that rebase, this would have a +1 or RTBC from me.

Thanks for the manual testing that is great. There is one comment early on in the other issue (#2) that claims the fix from here didn't work for them, but otherwise no-one has confirmed either way except @fjgarlin so let's go ahead and get this in.

I am not qualified to review the js here, but nod_ is and his feedback has been addressed.

Committed/pushed to 11.x, cherry-picked to 10.2.x, thanks!

Impressive discovery, test coverage and fix! 🤩👏

#3388913: Checkbox for Media library modal missing after search is another report related to this.