Skip to content

[Web] Re-enable wp-cron - #2039

Merged
adamziel merged 1 commit into
trunkfrom
reenable-wp-cron
Nov 29, 2024
Merged

adamziel merged 1 commit into
trunkfrom
reenable-wp-cron

Conversation

@adamziel

@adamziel adamziel commented Nov 29, 2024 •

Copy link
Copy Markdown
Collaborator

I disabled wp-cron a long time ago at a network bridge level with the following comment:

Disable wp-cron requests that are extremely slow in node.js runtime environment.
@todo: Make wp-cron requests faster.

That code path wasn't running in Node.js for months now. Also, Playground changed a lot since then and these requests no longer seem slow in my testing. This PR, thus, reinstantes wp-cron.

Related to #1749

Testing instructions

  1. Install wp-crontrol plugin and confirm on its settings page the schedules are running as expected. Confirm there are requests to wp-cron.php in devtools and that they're not slow.
  2. Try it in the web version of Playground (npm run dev)
  3. Try it in the CLI version of Playground (bun packages/playground/cli/src/cli.ts server) – although that one didn't even use the Wp_Http_Fetch_Base transport so nothing should change

I disabled wp-cron a long time ago at a network bridge level with the following comment:

> Disable wp-cron requests that are extremely slow in node.js runtime environment.
> @todo: Make wp-cron requests faster.

Playground changed a lot since then and these requests no longer seem
slow in my testing. This PR, thus, reinstantes wp-cron.

Related to #1749

 ## Testing instructions

1. Install wp-crontrol plugin and confirm on its settings page the
   schedules are running as expected. Confirm there are requests to
   wp-cron.php in devtools and that they're not slow.
2. Try it in the web version of Playground (`npm run dev`)
3. Try it in the CLI version of Playgroynd (`bun packages/playground/cli/src/cli.ts server`)
@adamziel adamziel changed the title Web: Re-enable wp-cron [Web] Re-enable wp-cron Nov 29, 2024
method: fetchMethod,
headers: fetchHeaders,
body: data.data,
body: fetchMethod === 'GET' ? undefined : data.data,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is a tangential bugfix – I noticed some requests failed with "GET requests cannot have body" when data.data was an empty string.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a tangential bugfix – I noticed some requests failed with "GET requests cannot have body" when data.data was an empty string.

Nice.

@adamziel
adamziel merged commit 6abb25f into trunk Nov 29, 2024
@adamziel
adamziel deleted the reenable-wp-cron branch November 29, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants