Skip to content

feat: Lazy-load Push adapter only when Push is configured#10522

Open
ga262 wants to merge 3 commits into
parse-community:alphafrom
ga262:feat/lazy-load-push-adapter
Open

feat: Lazy-load Push adapter only when Push is configured#10522
ga262 wants to merge 3 commits into
parse-community:alphafrom
ga262:feat/lazy-load-push-adapter

Conversation

@ga262

@ga262 ga262 commented Jun 24, 2026

Copy link
Copy Markdown

Issue

Closes #10518 — Parse Server loads the default Push adapter during startup even when Push notifications are not configured, adding unnecessary overhead for serverless, API-only, or test deployments.

Approach

Add an early return in getPushController() when options.push is not set. This skips loading the @parse/push-adapter module and initializing the push worker entirely, returning sensible defaults (hasPushSupport: false, no worker). When push is configured, existing behavior is fully preserved.

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check

Summary by CodeRabbit

  • Bug Fixes

    • Improved push notification handling so servers without push configured now correctly report push as unavailable.
    • When push is enabled, push support is detected more reliably and push-related behavior loads only when needed.
  • Tests

    • Added coverage for push loading and support detection in both enabled and disabled configurations.

Skip loading `@parse/push-adapter` and initializing push workers when
`options.push` is not set, reducing startup overhead for deployments
that don't use Push notifications.

Closes parse-community#10518

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@parse-github-assistant

Copy link
Copy Markdown

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Tip

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

Note

Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect.

Caution

Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 164fe8c6-2a97-4583-b7f2-eb4c3a5e2368

📥 Commits

Reviewing files that changed from the base of the PR and between 655220c and 16a28ad.

📒 Files selected for processing (1)
  • spec/index.spec.js

📝 Walkthrough

Walkthrough

getPushController now returns a no-push controller state immediately when options.push is absent, avoiding @parse/push-adapter loading. When push is configured, hasPushSupport is determined only by pushAdapter, and tests cover both paths.

Changes

Lazy push adapter initialization

Layer / File(s) Summary
Early-return and hasPushSupport simplification in getPushController
src/Controllers/index.js
When options.push is falsy, the function returns immediately with hasPushSupport: false, hasPushScheduledSupport: false, and pushWorker: undefined, skipping @parse/push-adapter loading entirely. In the push-enabled path, hasPushSupport is now !!pushAdapter instead of the previous two-condition check.
Push adapter loading tests
spec/index.spec.js
Tests assert that @parse/push-adapter is not loaded when push is undefined and that push support stays disabled, and that it is loaded when push is configured and push support becomes enabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Engage In Review Feedback ❓ Inconclusive Repo shows code/tests and one commit, but no PR review-thread evidence, so engagement with feedback can't be verified. Provide the PR review comments or thread links showing the author responded to feedback and either implemented it or got it retracted before resolution.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the PR scope and uses the required feat: prefix.
Description check ✅ Passed The description includes Issue, Approach, and Tasks sections and explains the change and tests.
Linked Issues check ✅ Passed The changes implement lazy loading when push is absent and preserve push-enabled behavior, matching #10518.
Out of Scope Changes check ✅ Passed The patch is limited to Push controller logic and its tests, with no unrelated changes visible.
Security Check ✅ Passed No vulnerable pattern found: the change only skips a static '@parse/push-adapter' import when push is disabled; no user-controlled loading, eval, or auth bypass was added.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/Controllers/index.js (1)

185-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplification of hasPushSupport is correct.

Since this path only runs when push is truthy (guarded by the early return at Line 175), dropping the previous coupling to push and relying solely on !!pushAdapter is equivalent. The Object.assign({}, push) copy before delete also avoids mutating the caller's options.push.

Optional: the queue-option extraction can be expressed more idiomatically with rest destructuring.

♻️ Optional: extract queueOptions via destructuring
-  const pushOptions = Object.assign({}, push);
-  const pushQueueOptions = pushOptions.queueOptions || {};
-  if (pushOptions.queueOptions) {
-    delete pushOptions.queueOptions;
-  }
+  const { queueOptions: pushQueueOptions = {}, ...pushOptions } = push;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Controllers/index.js` around lines 185 - 198, Refactor the push options
setup in `Controllers/index.js` to extract `queueOptions` more idiomatically
with object rest destructuring instead of cloning `pushOptions` and deleting
`queueOptions`. Keep the existing behavior in the `loadModule`, `loadAdapter`,
and `PushController` flow unchanged, and ensure `pushOptions` still excludes
`queueOptions` while `pushQueueOptions` preserves the extracted value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/Controllers/index.js`:
- Around line 185-198: Refactor the push options setup in `Controllers/index.js`
to extract `queueOptions` more idiomatically with object rest destructuring
instead of cloning `pushOptions` and deleting `queueOptions`. Keep the existing
behavior in the `loadModule`, `loadAdapter`, and `PushController` flow
unchanged, and ensure `pushOptions` still excludes `queueOptions` while
`pushQueueOptions` preserves the extracted value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79a0488b-9f80-4ff6-83b0-e5f545d3ba2c

📥 Commits

Reviewing files that changed from the base of the PR and between 4d3465c and 655220c.

📒 Files selected for processing (1)
  • src/Controllers/index.js

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 24, 2026
Assert that `@parse/push-adapter` is not loaded when `push` is not
configured, and is loaded when `push` is configured.

Co-Authored-By: Claude Opus 4.8 <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.

Lazy-load Push adapter only when Push is configured

1 participant