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

Remove WP_DEBUG from local-env till WP 6.3 is released #7591

Merged
merged 5 commits into from
Jul 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bin/local-env/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ services:
- wordpress_data:/var/www/html
- ../../:/var/www/html/wp-content/plugins/amp
- ./uploads.ini:/usr/local/etc/php/conf.d/uploads.ini
# @TODO: Remove it once Gutenberg 16.3 is released.
- /tmp/gutenberg/:/var/www/html/wp-content/plugins/gutenberg
depends_on:
- mysql

Expand All @@ -20,6 +22,8 @@ services:
volumes:
- wordpress_data:/var/www/html
- ../../:/var/www/html/wp-content/plugins/amp
# @TODO: Remove it once Gutenberg 16.3 is released.
- /tmp/gutenberg/:/var/www/html/wp-content/plugins/gutenberg
env_file:
- .env.wp
depends_on:
Expand Down
24 changes: 12 additions & 12 deletions bin/local-env/install-wordpress.sh
Original file line number Diff line number Diff line change
Expand Up @@ -108,25 +108,25 @@ wp plugin activate amp --quiet

# Install & activate Gutenberg plugin.
echo -e $(status_message "Installing and activating Gutenberg plugin...")
wp plugin install gutenberg --activate --force --quiet

# @TODO: Remove it once Gutnberg 16.3 is released.
# wp plugin install gutenberg --activate --force --quiet
wp plugin activate gutenberg --quiet
Comment on lines +111 to +114
Copy link
Member

Choose a reason for hiding this comment

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

While not critical since this is temporary, the wp plugin install command also allows you to pass a URL to a ZIP file. So this could have been done instead of modifying start.sh:

Suggested change
# @TODO: Remove it once Gutnberg 16.3 is released.
# wp plugin install gutenberg --activate --force --quiet
wp plugin activate gutenberg --quiet
# @TODO: Remove it once Gutnberg 16.3 is released.
# wp plugin install gutenberg --activate --force --quiet
wp plugin install https://amp-wp.org/wp-content/uploads/2023/07/gutenberg.zip --activate --force --quiet

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried it but it was adding a suffix in the plugin name something like gutenberg-Ad5fe and it was problematic.


# Set pretty permalinks.
echo -e $(status_message "Setting permalink structure...")
wp rewrite structure '%postname%' --hard --quiet

# Configure site constants.
echo -e $(status_message "Configuring site constants...")
WP_DEBUG_CURRENT=$(wp config get --type=constant --format=json WP_DEBUG | tr -d '\r')

if [ "$WP_DEBUG" != $WP_DEBUG_CURRENT ]; then
wp config set WP_DEBUG $WP_DEBUG --raw --type=constant --quiet
WP_DEBUG_RESULT=$(wp config get --type=constant --format=json WP_DEBUG | tr -d '\r')
echo -e $(status_message "WP_DEBUG: $WP_DEBUG_RESULT...")
fi
# @TODO: Remove this once WP 6.3 is released.
wp config delete WP_DEBUG

# Log WP_DEBUG value.
WP_DEBUG_CURRENT=$(wp config get --type=constant --format=json WP_DEBUG | tr -d '\r')
echo -e $(status_message "WP_DEBUG: $WP_DEBUG_CURRENT...")

# Log SCRIPT_DEBUG value.
SCRIPT_DEBUG_CURRENT=$(wp config get --type=constant --format=json SCRIPT_DEBUG | tr -d '\r')
if [ "$SCRIPT_DEBUG" != $SCRIPT_DEBUG_CURRENT ]; then
wp config set SCRIPT_DEBUG $SCRIPT_DEBUG --raw --type=constant --quiet
SCRIPT_DEBUG_RESULT=$(wp config get --type=constant --format=json SCRIPT_DEBUG | tr -d '\r')
echo -e $(status_message "SCRIPT_DEBUG: $SCRIPT_DEBUG_RESULT...")
fi
echo -e $(status_message "SCRIPT_DEBUG: $SCRIPT_DEBUG_CURRENT...")
5 changes: 5 additions & 0 deletions bin/local-env/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ cd "$(dirname "$0")/../.."
# Check whether Composer installed
. "$(dirname "$0")/install-composer.sh"

# @TODO: Remove it once Gutnberg 16.3 is released.
rm -rf /tmp/gutenberg /tmp/gutenberg.zip
wget https://amp-wp.org/wp-content/uploads/2023/07/gutenberg.zip -O /tmp/gutenberg.zip
unzip /tmp/gutenberg.zip -d /tmp/gutenberg

# Check whether Docker is installed and running
. "$(dirname "$0")/launch-containers.sh"

Expand Down
9 changes: 9 additions & 0 deletions includes/embeds/class-amp-core-block-handler.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ class AMP_Core_Block_Handler extends AMP_Base_Embed_Handler {
* Register embed.
*/
public function register_embed() {
/*
* Disable interactivity API on core/navigation block.
* Currently this support is added by Gutenberg plugin, but it will be a part of WP 6.3 as well.
*
* @TODO: Need to revisit once Interactivity API is landed in WP Core.
*/
add_filter( 'gutenberg_should_block_use_interactivity_api', '__return_false' );
Comment on lines +69 to +75
Copy link
Member

Choose a reason for hiding this comment

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

If this is added here, then the inverse should be done in unregister_embed.

Nevertheless, the gutenberg_should_block_use_interactivity_api filter passes a second argument which is the block name. Should this be checked to only disable the Navigation block?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nevertheless, the gutenberg_should_block_use_interactivity_api filter passes a second argument which is the block name. Should this be checked to only disable the Navigation block?

Do we need interactivity API enabled on any block in our case? Since this API will be added in more blocks and those modifications could be problematic, I believe we should keep it disabled for all blocks.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good


add_filter( 'render_block', [ $this, 'filter_rendered_block' ], 0, 2 );
add_filter( 'widget_text_content', [ $this, 'preserve_widget_text_element_dimensions' ], PHP_INT_MAX );
}
Expand All @@ -74,6 +82,7 @@ public function register_embed() {
* Unregister embed.
*/
public function unregister_embed() {
remove_filter( 'gutenberg_should_block_use_interactivity_api', '__return_false' );
remove_filter( 'render_block', [ $this, 'filter_rendered_block' ], 0 );
remove_filter( 'widget_text_content', [ $this, 'preserve_widget_text_element_dimensions' ], PHP_INT_MAX );
}
Expand Down