FIX: Litespeed Cache plugin compatibility https://github.com/Crocoblock/jetformbuilder/issues/603

[Gawuww]

Issue

• Litespeed Cache plugin compatibility #603

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR changes the Litespeed compatibility check in compatibility/litespeed/litespeed.php::condition() from a simple LSCWP_V constant check to also verify the Litespeed ESI status via apply_filters('litespeed_esi_status', false).

What I looked at

• File: compatibility/litespeed/litespeed.php
• Function: public function condition(): bool

Positives

• The change addresses a real bug: the presence of the LSCWP_V constant does not guarantee that Litespeed ESI (Edge Side Includes) is enabled. Checking the ESI status avoids erroneously enabling compatibility when ESI is not active.
• The implementation is simple and avoids heavy runtime work (single apply_filters call).

Potential issues / suggestions

1. Behavior change / backward compatibility

• The previous implementation returned true when LSCWP_V was defined. Now the check requires the litespeed_esi_status filter to return truthy. This is an intentional behaviour change (fix), but it may cause different runtime paths on sites that previously relied on the old check. This is acceptable for a compatibility fix, but call it out in changelog (done below).

2. apply_filters timing / plugin initialization

• apply_filters('litespeed_esi_status', false) is fine—if Litespeed cache hasn't added that filter yet, the default false will be returned which is safe. However, if this method may run very early (during plugin load) before Litespeed has registered its filter, you might mistakenly conclude ESI is disabled when it is in fact enabled. You already guard by checking the LSCWP_V constant, but that constant may be present before the filter is registered.
• Optional improvement: check for the filter presence to make the intent explicit (has_filter), or check for an Litespeed helper function if one exists (for example, if Litespeed exposes a function or class method to check ESI). Example:
  if ( defined( 'LSCWP_V' ) && has_filter( 'litespeed_esi_status' ) ) {
  return (bool) apply_filters( 'litespeed_esi_status', false );
  }
  return false;
  This makes the timing dependency explicit and avoids running the filter when no listener is present.

3. Namespacing & safety

• Using apply_filters with a default of false is safe. No escaping/sanitization issues because this is a boolean check internal to logic. No user input involved.

4. Tests

• I could not find unit/integration tests for this compatibility class. Please consider adding a small unit/integration test that simulates the presence of LSCWP_V and toggles the litespeed_esi_status filter to assert that condition() returns expected values. This is low-effort and helpful to avoid regressions.

5. Docblock / comment

• Consider adding a one-line comment above the check explaining why we now consult the litespeed_esi_status filter (clarify the difference between LSCWP_V presence and ESI being active). That helps future maintainers.

Conclusion

• Change is appropriate and fixes the issue reported in Litespeed Cache plugin compatibility #603. Minor improvement: consider using has_filter to make timing explicit. Add a unit/integration test and a short inline comment. No security concerns found.

Suggested changelog entry

- FIX: Litespeed Cache compatibility — only enable compatibility when Litespeed ESI is active (uses litespeed_esi_status filter).

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• The PR tightens the Litespeed compatibility check in compatibility/litespeed/litespeed.php by requiring both the LSCWP_V constant and the litespeed_esi_status filter to return truthy. Previously the code only checked for LSCWP_V.

What I looked at

• File: compatibility/litespeed/litespeed.php
• Method: public function condition()
• Change: return (bool) apply_filters( 'litespeed_esi_status', false ); after confirming LSCWP_V is defined

Positive

• Makes the compatibility detection more specific by ensuring ESI is actually enabled (via the litespeed_esi_status filter) instead of assuming any presence of the Litespeed plugin implies that behavior. This should reduce false positives.
• No obvious performance or security issues introduced (apply_filters is cheap and safe here).

Concerns / Suggestions

1. Backward compatibility risk

   • Behaviour change: previously JetFormBuilder considered Litespeed present when LSCWP_V was defined. Now compatibility will be disabled if the litespeed_esi_status filter is not hooked or returns false. That could unexpectedly disable compatibility for sites that relied on the previous behavior (for example older Litespeed versions or setups that do not expose that filter). This is a functional change that may affect users.
   • Suggestion: preserve backwards compatibility by treating presence of the constant as an enabling condition when the filter is not present. e.g.:
     if ( ! defined( 'LSCWP_V' ) ) {
     return false;
     }
     if ( ! has_filter( 'litespeed_esi_status' ) ) {
     return true; // preserve previous behavior if filter is not available
     }
     return (bool) apply_filters( 'litespeed_esi_status', false );
   • This keeps the stricter check when the Litespeed plugin exposes the filter, while not breaking older setups.

2. Use of has_filter() vs relying on default

   • Using has_filter() makes the intent explicit and documents fallback behaviour. If you intentionally want to disable compatibility unless the site explicitly signals ESI via the filter, keep current code but document this breaking change in changelog and mention it in the GH issue.

3. Tests and changelog

   • Add a small unit/integration test or a manual QA checklist verifying both scenarios: (a) Litespeed present and litespeed_esi_status returns true -> compatibility enabled, and (b) Litespeed present but filter absent or returns false -> compatibility disabled (or preserved depending on chosen fallback). This is higher risk because it affects behavior when an external plugin is present.
   • Add a changelog entry that clearly states the compatibility detection logic changed (so admins know why compatibility might be disabled after update).

4. Multisite considerations

   • ESI may be configurable per-site. The filter-based approach should respect per-site settings if the Litespeed plugin applies the filter per-blog. Confirm this behavior in multisite environments during QA.

5. Minor: code comment

   • Add an inline comment explaining why the filter is used (detect ESI mode) so future maintainers understand the rationale.

Overall recommendation

• The change improves correctness but may break older deployments. Please either add a has_filter() fallback to preserve previous behavior or document this as a breaking/behavioral change and add tests/QA notes. Also add a changelog entry and a short inline comment in the file explaining why litespeed_esi_status is checked.

Suggested changelog entry

- FIX: Litespeed Cache compatibility — only enable compatibility when Litespeed ESI is active (use litespeed_esi_status), avoiding false positives