Skip to content

fix: Add type checks before wrapping wp.ajax methods#282

Merged
dcalhoun merged 1 commit intofeat/authorize-ajax-with-application-passwordsfrom
claude/pr181-race-condition-fix-EoBab
Jan 14, 2026
Merged

fix: Add type checks before wrapping wp.ajax methods#282
dcalhoun merged 1 commit intofeat/authorize-ajax-with-application-passwordsfrom
claude/pr181-race-condition-fix-EoBab

Conversation

@dcalhoun
Copy link
Copy Markdown
Member

What?

Add type checks before wrapping wp.ajax.send and wp.ajax.post methods in the AJAX configuration.

Why?

This addresses PR review feedback about a potential race condition. If window.wp.ajax.send or window.wp.ajax.post are undefined when configureAjax() executes, the wrapped function would throw a TypeError when invoked (calling .call() on undefined).

References: #181

How?

  • Added typeof window.wp.ajax.send === 'function' check before wrapping the send method
  • Added typeof window.wp.ajax.post === 'function' check before wrapping the post method
  • Methods are only wrapped if they exist as functions, preventing runtime errors
  • Updated tests to verify that missing methods remain undefined rather than being wrapped with an undefined reference

Testing Instructions

  1. Run the test suite: npm run test:unit
  2. Verify all 20 tests in ajax.test.js pass
  3. Specifically check the "should handle missing wp.ajax.send method" and "should handle missing wp.ajax.post method" tests

Accessibility Testing Instructions

N/A - This is a JavaScript utility change with no UI impact.

Screenshots or screencast

N/A - No visual changes.

Address PR feedback about potential race condition. The code now checks
if `window.wp.ajax.send` and `window.wp.ajax.post` are functions before
wrapping them. This prevents TypeError when calling the wrapped function
if the original method was undefined during configuration.

Update tests to verify that missing methods remain undefined rather than
being wrapped with an undefined reference.
@dcalhoun dcalhoun marked this pull request as ready for review January 14, 2026 20:03
@dcalhoun dcalhoun merged commit 394bce2 into feat/authorize-ajax-with-application-passwords Jan 14, 2026
8 checks passed
@dcalhoun dcalhoun deleted the claude/pr181-race-condition-fix-EoBab branch January 14, 2026 20:03
dcalhoun added a commit that referenced this pull request Feb 12, 2026
Address PR feedback about potential race condition. The code now checks
if `window.wp.ajax.send` and `window.wp.ajax.post` are functions before
wrapping them. This prevents TypeError when calling the wrapped function
if the original method was undefined during configuration.

Update tests to verify that missing methods remain undefined rather than
being wrapped with an undefined reference.

Co-authored-by: Claude <noreply@anthropic.com>
dcalhoun added a commit that referenced this pull request Mar 24, 2026
Address PR feedback about potential race condition. The code now checks
if `window.wp.ajax.send` and `window.wp.ajax.post` are functions before
wrapping them. This prevents TypeError when calling the wrapped function
if the original method was undefined during configuration.

Update tests to verify that missing methods remain undefined rather than
being wrapped with an undefined reference.

Co-authored-by: Claude <noreply@anthropic.com>
dcalhoun added a commit that referenced this pull request Mar 26, 2026
Address PR feedback about potential race condition. The code now checks
if `window.wp.ajax.send` and `window.wp.ajax.post` are functions before
wrapping them. This prevents TypeError when calling the wrapped function
if the original method was undefined during configuration.

Update tests to verify that missing methods remain undefined rather than
being wrapped with an undefined reference.

Co-authored-by: Claude <noreply@anthropic.com>
dcalhoun added a commit that referenced this pull request Mar 31, 2026
* feat: Authorize AJAX with application passwords

Include authorization header in AJAX requets, as we do not have cookies
to send in the mobile app environment.

* refactor: Rename AJAX and api-fetch configuration utilities

* fix: Configure AJAX after the library loads

If we configure AJAX before loading the library, the configuration is
overridden.

* test: Fix test imports and assertions

* fix: Set the global WordPress admin AJAX URL

This global is often used by WordPress Admin page scripts.

* test: Assert AJAX configuration

* feat: Allow configuring the Android asset loader domain

Useful when needing to allow CORS for specific domains.

* docs: Note AJAX support requirements

* docs: Note AJAX CORS errors in troubleshooting documentation

* fix: Add type checks before wrapping wp.ajax methods (#282)

Address PR feedback about potential race condition. The code now checks
if `window.wp.ajax.send` and `window.wp.ajax.post` are functions before
wrapping them. This prevents TypeError when calling the wrapped function
if the original method was undefined during configuration.

Update tests to verify that missing methods remain undefined rather than
being wrapped with an undefined reference.

Co-authored-by: Claude <noreply@anthropic.com>

* feat: conditionally reinstate VideoPress bridge

When `videopress/video` is not in `allowed_block_types`, initialize the
VideoPress AJAX bridge to handle `core/video` blocks extended to rely
upon VideoPress upload services. AJAX auth is always initialized.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: filter lodash-js-after inline script from editor assets

WordPress's `lodash-js-after` inline script calls `_.noConflict()` to
restore `window._` to Underscore.js. Since GutenbergKit excludes core
WordPress assets from the editor assets endpoint but doesn't load
Underscore, this wipes `window._` to `undefined`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: vendor and load wp-util.js

GutenbergKit excludes core WordPress assets from the editor assets
endpoint, so wp-util.js (which provides wp.ajax and wp.template) must
be vendored and loaded directly. Load it via dynamic import at the end
of initializeWordPressGlobals() after jQuery and lodash are on window,
since its IIFE captures jQuery via closure at execution time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: align wp.ajax wrapper signatures with wp-util

The wp.ajax.send and wp.ajax.post wrappers accepted a single options
argument, but wp-util's implementation accepts (action, options). Align
the wrapper signatures so the action argument is forwarded correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use home URL for iOS demo app site URL

Use `homeUrlString()` instead of `siteUrlString()` from the REST API
root response. The `url` field often returns `http://` for WordPress.com
sites, while `home` returns the actual public-facing `https://` URL.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* feat: alias wp.media.ajax and wp.media.post to wp.ajax

WordPress core sets these aliases in media-models.js, which
GutenbergKit doesn't load. Alias them after auth wrapping so media
uploads use the authenticated AJAX methods.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* build: Use wp-util from production WordPress release

Avoid including latest changes from the WordPress/wordpress-develop
repository.

* docs: Expand inline comments

* refactor: scope AJAX auth to same-site requests via ajaxPrefilter

Replace jQuery.ajaxSetup and wp.ajax.send/post wrappers with a single
jQuery.ajaxPrefilter that only injects the Authorization header when
the request URL starts with the configured siteURL. This prevents
leaking credentials to cross-origin requests and avoids argument
normalization issues with the previous wp.ajax wrapper approach.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: strip trailing slash from siteURL before building AJAX URLs

Prevents double-slash in constructed URLs (e.g.,
`https://example.com//wp-admin/admin-ajax.php`) when siteURL is
provided with a trailing slash.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: warn instead of logging success when jQuery is unavailable for AJAX auth

The ajaxPrefilter silently no-ops via optional chaining when jQuery is
missing, but the debug log still claims auth was configured. Guard with
an early return and warning so the log accurately reflects what happened.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: use origin-based matching for AJAX auth header injection

Replace `startsWith(siteURL)` with `URL.origin` comparison so that
scheme, host, and port must all match exactly. This prevents credential
leakage to lookalike domains (e.g. `https://example.com.evil.com`).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: separate AJAX config examples from Android requirement

Move the iOS and Android code examples out of the Android-specific
requirement so they are not visually nested under that bullet point.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: remove redundant AJAX setup from VideoPress bridge

configureAjax() now initializes wp.ajax, wp.ajax.settings, and the AJAX
URL before the VideoPress bridge runs, making the duplicate setup in
initializeVideoPressAjaxBridge() unnecessary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add defensive guards to AJAX configuration

- Wrap `new URL(siteURL)` in try/catch so a malformed siteURL logs a
  warning instead of throwing.
- Guard `configureMediaAjax` against missing `wp.ajax.send`/`post`
  (e.g., if wp-util.js failed to load).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: validate assetLoaderDomain in Android EditorConfiguration

Throw IllegalArgumentException if the value contains a scheme, path, or
is blank, so callers get a clear error instead of a malformed asset URL
at runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add vendor README documenting wp-util.js source

Record the upstream commit hash and rationale for vendoring so future
maintainers know where the file came from and when to update it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: add https scheme to WP.com site URL in Android demo app

Account.WpCom.username stores just the hostname (e.g.,
"dcpaid.wordpress.com") since it is extracted via URI.host during OAuth.
ConfigurationItem was using this bare hostname as siteUrl, producing
invalid AJAX endpoints. Prepend "https://" to match the self-hosted
flow, which receives a full URL from the callback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: extract duplicated AJAX URL into local variable

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: remove lint/format exclusion note from vendor README

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: derive Android WebView asset domain from site URL

Derive the WebViewAssetLoader domain from the configured siteURL
instead of defaulting to the synthetic appassets.androidplatform.net
domain. This makes REST API and admin-ajax.php requests same-origin,
eliminating CORS restrictions without requiring server-side headers.

- Restrict shouldOverrideUrlLoading to /assets/ paths on the asset
  domain so arbitrary site pages don't load inside the WebView.
- Reorder shouldInterceptRequest to check the cache interceptor
  before the asset loader, preventing cached JS/CSS from being
  short-circuited when both share the site domain.
- Remove the now-unnecessary assetLoaderDomain configuration option
  from EditorConfiguration.
- Update AJAX documentation to reflect the simplified setup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

2 participants