Skip to content

refactor: renderer fix, remove page.js, shared base classes, reactive waitOn#127

Merged
dr-dimitru merged 7 commits into
veliovgroup:masterfrom
dupontbertrand:refactor/core-ready
Apr 21, 2026
Merged

refactor: renderer fix, remove page.js, shared base classes, reactive waitOn#127
dr-dimitru merged 7 commits into
veliovgroup:masterfrom
dupontbertrand:refactor/core-ready

Conversation

@dupontbertrand

Copy link
Copy Markdown

Summary

This PR bundles 5 focused improvements identified while integrating flow-router-extra into Meteor core as a first-class package. All 147 tests pass.

Commits

  • fix: renderer queue — startQueue.bind() was never called
    BlazeRenderer.startQueue() called .bind(this) but never invoked the result, so queued render tasks were silently dropped. Fixed to use requestAnimFrame(() => this.startQueue()).

  • security: remove publicly exposed ___refresh route
    The FlowRouter.route('/___refresh/:layout/:template/:oldRoute?', {...}) handler called JSON.parse(queryParams.oldParams) without validation on a publicly accessible URL. Removed the route and the FlowRouter.refresh() method entirely.

  • fix: replace setTimeout polling in waitOn with Tracker.autorun
    The previous implementation polled subscription readiness every N ms with exponential backoff (up to 1000 ms). Replaced with a Tracker.autorun that resolves a Promise as soon as sub.ready() becomes true — no unnecessary delays, no wasted CPU.

  • refactor: extract RouterBase, RouteBase, GroupBase shared base classes
    Client and server implementations shared ~300 lines of duplicated logic. Extracted into lib/router-base.js, lib/route-base.js, lib/group-base.js. Client and server now extend these, keeping environment-specific code only where needed.

  • feat: replace page.js with custom MicroRouter
    page.js (v1.9.0, last release 2019, 2000 lines) is replaced by a 335-line MicroRouter in lib/micro-router.js. Same feature set: pushState/replaceState, popstate, click interception, base path support. Also fixes a double base-path bug present in the original integration and a redirect-from-exit recursion bug (via _isRedirecting flag). Removes the page npm dependency.

Test plan

  • 147/147 tests pass (meteor test-packages ./)
  • All 5 commits are independent and reviewable separately
  • No changes to public API

`this.startQueue.bind(this)` creates a new bound function but does not
invoke it, so queued render tasks after the first were silently dropped.

Replace with `requestAnimFrame(() => this.startQueue())` which actually
schedules and calls the next iteration of the queue.
The /___refresh/:layout/:template/:oldRoute? route was registered
automatically at startup with no authentication or validation.

It called JSON.parse(queryParams.oldParams) on untrusted user input,
and passed :layout and :template params directly to this.render()
without checking they correspond to existing templates.

Use FlowRouter.reload() to reload the current route instead.

BREAKING CHANGE: FlowRouter.refresh(layout, template) is removed.
The previous implementation used a cascade of Meteor.setTimeout calls
(12ms → 24ms → 128ms → 256ms) with a waitFails counter that silently
reset after 9 failures. No timer cleanup was performed on exit.

Replace with:
- await Promise.all() for promise-based waitOn entries
- Tracker.autorun that re-runs reactively only when sub.ready() changes,
  resolving a Promise when all subscriptions are ready

This eliminates busy-waiting, removes the unpredictable timer cascade,
and integrates naturally with Meteor's reactivity system.
Client and server implementations duplicated ~300 lines of path
generation, route registration, group logic, and subscription handling.

Extract three shared base classes into lib/:
- RouterBase: path(), url(), group(), onRouteRegister(),
  _triggerRouteRegister(), common constructor state, client-only stubs
- RouteBase: subscription map (register, getSubscription,
  getAllSubscriptions, clearSubscriptions, callSubscriptions)
- GroupBase: full Group implementation (superset of client and server)

Server router now extends RouterBase and keeps only server-specific
methods: route(), matchPath(), setCurrent().
Server route and group are now thin wrappers around the base classes.

No behavior change — public API is identical.
page.js v1.9.0 (last commit 2019, ~2000 lines) is replaced by a
custom MicroRouter in lib/micro-router.js (~335 lines).

Improvements:
- No more monkey-patching of page.show / page.replace in initialize()
- Single decodeURIComponent — no double-decoding bug and no char-by-char
  double-encoding workaround in path generation
- Clean History API integration (pushState, replaceState, popstate)
- <a> click interception handles same-origin links only, with proper
  attribute checks (download, target, rel=external)
- No external dependency — pure ES2015 class, no npm package

server/router.js: matchPath() now uses pathToRegExp/matchPath from
lib/micro-router.js instead of page.Route, removing the last server-side
usage of page.js. Route patterns are compiled once and cached per route
object via a WeakMap.

package.js: removes page: '1.9.0' from Npm.depends.

BREAKING CHANGE: FlowRouter.initialize() no longer accepts a `page`
options object (page.js-specific options). The `hashbang`,
`decodeURLComponents`, and `window` options are removed.
`click` and `popstate` are still supported.
…erence

FlowRouter._qs was removed when page.js was replaced by MicroRouter.
pathFor() used FlowRouter._qs.parse() for query string parsing — now
imports qs directly from ./modules.js instead.
@dupontbertrand dupontbertrand changed the title refactor: core-ready improvements — renderer fix, remove page.js, shared base classes, reactive waitOn refactor: renderer fix, remove page.js, shared base classes, reactive waitOn Mar 11, 2026
@dupontbertrand dupontbertrand marked this pull request as ready for review March 13, 2026 08:33
@dupontbertrand

Copy link
Copy Markdown
Author

Cc @dr-dimitru 👀 🙏

@dr-dimitru

Copy link
Copy Markdown
Member

@dupontbertrand looks great, thank you for preparing this one. I had a plan to move it off the page.js to native "URL Pattern API" for the far too long. I'll review it shortly

@dupontbertrand

Copy link
Copy Markdown
Author

Hey boss ! Do you have time to take care of that ? 🙏

@dr-dimitru

Copy link
Copy Markdown
Member

@dupontbertrand working on it now. required some refactoring

@dupontbertrand

Copy link
Copy Markdown
Author

@dupontbertrand working on it now. required some refactoring

Do you want to comment the changes required ? I can do them, as you want 🙏

@dr-dimitru dr-dimitru merged commit 88e75a8 into veliovgroup:master Apr 21, 2026
dr-dimitru added a commit that referenced this pull request Apr 21, 2026
# ostrio:flow-router-extra Changelog

For full history see [GitHub releases](https://github.com/veliovgroup/flow-router/releases).

## v3.14.0 (2026-04-22)

### ⚠️ Major changes
- Replaced page.js w/ custom `MicroRouter` (`lib/micro-router.js`, client/router.js) — full control over history, popstate, matchPath; removes dep, fixes bugs. (PRs #126/#127), closing #74 and #73, thanks to @dupontbertrand
- Shared base classes (`lib/router-base.js` etc) for Router/Route/Group — isomorphic, cleaner code, better maintenance.
- Async `waitOn` overhaul: correct hook order, completeness, abort on navigation, Tracker.autorun instead of setTimeout polling. **Highlights new robust async feature**.
- Removed `underscore` dep (refactor to native methods; updated tests/docs).

### Changes
- 🔧 Fixed order and completeness in async waitOn (client/route.js, router.core.spec.js).
- 🔧 Fixed undefined `name` in tests; added missed `timer` var.
- 🛡️ Security: removed publicly exposed `___refresh` route.
- 🔧 Fixed renderer queue (startQueue.bind() issue).
- 📔 Docs updated to prefer native JS; AGENTS.md + SKILL.md for ecosystem.
- 👷‍♂️ TS refactor/setup, .cursorignore, styling, year bump, test helpers.
- notfound route workaround/fixes for companion packages compatibility (meta/title).

### ✨ New
- 😎 Agentic Skills, AGENTS.md for Cursor IDE + meteor-flow-router (registration, hooks, meta/title, TS, 404).
- 👨‍💻 TS tests via `tsd` (`index.test-d.ts`); updated `index.d.ts`.
- Enhanced `FlowRouter.initialize(options)`, `maxWaitFor`, `MAX_WAIT_FOR_MS` export, onRouteRegister.
- Better Meteor 3.x, SWC/compiler compat, peer version alignment ([email protected]+ pins in meta).

### 📦 Dependencies
**prod**
- `qs`: 6.14.0 (from 6.x)

**dev**
- zodern:[email protected], typescript (weak); removed underscore from onTest where possible.
- Updated test-packages for current Meteor.
@dr-dimitru

Copy link
Copy Markdown
Member

@dupontbertrand please see v3.13.0

@dupontbertrand

Copy link
Copy Markdown
Author

Hey @dr-dimitru
Two small follow-ups I noticed while reviewing:

1.Client .path() uses a plain encodeURIComponent whereas server goes through _encodeParam (double-encode for / % +) it's intentional?
2.Happy to open a follow-up PR if you'd like MicroRouter migrated to the URL Pattern API you mentioned.

@dr-dimitru

Copy link
Copy Markdown
Member

@dupontbertrand

  1. It should be controlled by legacy option decodeQueryParamsOnce. Perhaps we forgot to apply it to Server

  2. No, this was my plan, but after some research I realized we won't be able to make it backward compatible. And it may be not available in all browsers, so it will require us to implement fallback, so it doesn't really makes sense. Let me know what do you think

PRs are welcome

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