Skip to content

Fine-grained disable the block editor - #2496

Draft
JPry wants to merge 34 commits into
bucket/editorfrom
classy/fine-grained-disable
Draft

Fine-grained disable the block editor#2496
JPry wants to merge 34 commits into
bucket/editorfrom
classy/fine-grained-disable

Conversation

@JPry

@JPry JPry commented Apr 12, 2025

Copy link
Copy Markdown
Contributor
  • Add initial block editor controller
  • Add methods for block loading for post type
  • Use ::class to reference the registered classes
  • Register the Block_Logic provider
  • Mark the editor class as deferred
  • Remove no-op method
  • Tweak Regexes
  • Correct docblock parameters
  • Simplify dynamic block stripping
  • Fix typo
  • Use interpolation and clean up some methods
  • Apply formatting to the file

🎫 Ticket

[TICKET_ID]

🗒️ Description

🎥 Artifacts

✔️ Checklist

  • Ran npm run changelog to add changelog file(s). More info here
  • Code is covered by NEW wpunit or integration tests.
  • Code is covered by EXISTING wpunit or integration tests.
  • Are all the required tests passing?
  • Automated code review comments are addressed.
  • Have you added Artifacts?
  • Check the base branch for your PR.
  • Add your PR to the project board for the release.

@JPry JPry self-assigned this Apr 12, 2025
@tec-bot

tec-bot commented Apr 16, 2025

Copy link
Copy Markdown
Contributor

PR to update Action Scheduler version to match WooCommerce version has been created: #2514

Comment thread src/Common/Editor/Block_Logic.php Outdated
Comment thread src/Common/Editor/Block_Logic.php Outdated
Comment thread src/Common/Editor/Block_Logic.php Outdated
*/
public function should_load_blocks_for_post_type( string $post_type = '' ): bool {
// Set up a static variable to ensure we don't check the same post type multiple times.
static $checked_post_types = [];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use tribe_cache()[ $cache_key ] (ArrayAccess API) for memoization. Static variables will not flush on wp_cache_flush calls making programmatic cache invalidation difficult (includes test, but not only those).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the topic of memoization: is this worth it? Are there so many calls to this method to justify it and run the risk of serving a stale result (e.g. because the post types have been filtered later)?
It looks like the expensive call in this method is the one to $this->determine_current_post_type(); should that method memoized?
I'm asking this question as I do not have an order of how many calls are made to this method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On the topic of memoization: is this worth it? Are there so many calls to this method to justify it and run the risk of serving a stale result (e.g. because the post types have been filtered later)?

My thought process here was that every place that calls should_load_blocks() could potentially get to this method as well. Because this method runs apply_filters(), and that isn't necessarily a light call, I wanted to minimize the number of times the logic needs to run.

With that said, I can switch to tribe_cache() instead.

I'm asking this question as I do not have an order of how many calls are made to this method.

At the moment it's only called in one place. However, I was trying to be defensive about it ahead of time, knowing how the calls are likely to expand over time.

}

// Try to use the global post object.
if ( isset( $GLOBALS['post']->post_type ) ) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this work reliably in REST API and Ajax context?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will this work reliably in REST API and Ajax context?

That's a good question: I think it would make sense to have a separate case for those situations outside of this method, since the post type isn't as relevant in those contexts.

Comment thread src/Common/Editor/Block_Logic.php Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants