Skip to content

Conversation

@vicb
Copy link
Owner

@vicb vicb commented Jan 3, 2026

Summary by Sourcery

Inline helper utilities into the custom ion-router component to remove the dependency on @ionic/core helpers and restore correct router behavior.

Bug Fixes:

  • Resolve ion-router malfunctions caused by relying on external @ionic/core helper imports.

Enhancements:

  • Inline componentOnReady and debounce helper implementations directly in the ion-router component for greater resiliency against upstream changes.

Summary by CodeRabbit

  • Refactor
    • Refactored internal utilities in the router component to improve code organization and maintainability.

✏️ Tip: You can customize this high-level summary in your review settings.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 3, 2026

Reviewer's Guide

Inlines the previously imported Ionic helper utilities componentOnReady and debounce directly into the custom ion-router wrapper to remove the external dependency on @ionic/core/components/helpers.js while preserving the original behavior.

File-Level Changes

Change Details Files
Inline Ionic helper utilities used by the custom ion-router component instead of importing them from the Ionic helpers module.
  • Remove the import of componentOnReady and debounce from @ionic/core/components/helpers.js.
  • Add an inlined raf helper that uses requestAnimationFrame when available and falls back to setTimeout.
  • Add an inlined componentOnReady implementation that either waits for the component’s own componentOnReady promise or schedules a callback via raf.
  • Add an inlined debounce utility that wraps a function and delays execution with setTimeout while clearing any pending timer.
apps/fxc-front/src/app/components/ui/ion-router.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link

coderabbitai bot commented Jan 3, 2026

Walkthrough

This change inlines utility function implementations (raf, componentOnReady, and debounce) directly into the ion-router component, removing external imports while preserving existing behavior. The local implementations maintain equivalent functionality for component readiness checks and debounce operations.

Changes

Cohort / File(s) Summary
Inline utility implementations
apps/fxc-front/src/app/components/ui/ion-router.ts
Replaces external imports of componentOnReady and debounce with local implementations. Introduces raf (requestAnimationFrame fallback), componentOnReady (wrapper for el.componentOnReady() with raf fallback), and debounce (timer-based utility). Preserves existing usage patterns while reducing external dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit hops through code with glee,
Pulling dependencies down from the tree,
Utils now local, dependencies freed,
Simpler, faster—all that we need! ✨
Thump, thump, thump—the refactor's complete!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix ion-router' is vague and generic, failing to specify what problem was fixed or what changes were made to the ion-router component. Clarify the title to specify the actual fix, such as 'Replace external utility imports with local implementations in ion-router' or 'Fix ion-router component readiness and debounce handling'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vicb/fixes

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 and usage tips.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The inlined debounce helper drops the original this binding when invoking func; consider calling func.apply(this, args) inside the returned function to preserve context.
  • The newly inlined helpers (componentOnReady, debounce, raf) are typed with any; tightening these types (e.g., explicit element and callback signatures) will improve type safety and catch potential misuse.
  • Since the helpers are now inlined instead of imported from @ionic/core, consider adding a shared local helper module so future upstream changes to these utilities can be updated in a single place rather than being duplicated here.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The inlined `debounce` helper drops the original `this` binding when invoking `func`; consider calling `func.apply(this, args)` inside the returned function to preserve context.
- The newly inlined helpers (`componentOnReady`, `debounce`, `raf`) are typed with `any`; tightening these types (e.g., explicit element and callback signatures) will improve type safety and catch potential misuse.
- Since the helpers are now inlined instead of imported from `@ionic/core`, consider adding a shared local helper module so future upstream changes to these utilities can be updated in a single place rather than being duplicated here.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@cloudflare-workers-and-pages
Copy link

Deploying flyxc with  Cloudflare Pages  Cloudflare Pages

Latest commit: 521c899
Status: ✅  Deploy successful!
Preview URL: https://2b7c194c.flyxc.pages.dev
Branch Preview URL: https://vicb-fixes.flyxc.pages.dev

View logs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/fxc-front/src/app/components/ui/ion-router.ts (1)

18-24: Minor inconsistency in callback parameter.

The two code paths pass slightly different values to the callback:

  • Line 20: passes resolvedEl (the element returned by componentOnReady())
  • Line 22: passes el (the original element)

This is likely intentional since Stencil's componentOnReady() typically returns the same element after initialization. However, for perfect consistency, line 22 could also use resolvedEl naming or ensure the values are truly equivalent.

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 656fe7f and 521c899.

📒 Files selected for processing (1)
  • apps/fxc-front/src/app/components/ui/ion-router.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Sourcery review
  • GitHub Check: build (22.x)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
apps/fxc-front/src/app/components/ui/ion-router.ts (3)

12-17: LGTM with a minor observation.

The raf implementation is correct for its use case. Note that setTimeout returns a timeout ID while requestAnimationFrame returns a request ID—these cannot be cancelled the same way. However, since the return value is never captured or used in this codebase (line 22), this inconsistency has no practical impact.


26-32: LGTM!

The debounce implementation is correct and follows standard patterns. The timer is properly cleared on each invocation, and arguments are correctly forwarded. The usage at line 430 properly binds the context with .bind(this).


11-32: The inlined implementations appear sound for their use cases; external verification would require accessing the original Ionic Framework source.

Since this code is sourced from an external project (ionic-framework/core/components/ion-router.js), the original implementations cannot be compared within this repository. The inlined functions follow standard JavaScript patterns and are used correctly in context:

  • raf provides appropriate fallback for animation frame requests
  • componentOnReady correctly awaits component initialization with Promise
  • debounce properly implements trailing-edge debouncing for event listeners

If concerned about behavioral parity with the Ionic Framework version, consider referencing the ionic-framework repository directly or consulting its changelog for the "my-router branch" mentioned in the file header.

@vicb vicb merged commit e473026 into master Jan 3, 2026
7 checks passed
@vicb vicb deleted the vicb/fixes branch January 3, 2026 11:31
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