feat(journeys,core): fix defineModule + derive step ordering/progress (feedback items 3 & 4) - #92
Closed
kibertoad wants to merge 12 commits into
Closed
feat(journeys,core): fix defineModule + derive step ordering/progress (feedback items 3 & 4)#92kibertoad wants to merge 12 commits into
kibertoad wants to merge 12 commits into
Conversation
… (feedback items 3 & 4)
Implements the next two items from docs/consumer-feedback-production-app.md.
Item 3 — defineModule usable by real apps (all five variants:
@modular-frontend/core + the four router cores):
- Infer a trailing TDescriptor from the argument and return it verbatim, so
entryPoints/exitPoints keep their literal keys instead of widening. typeof
someModule now drops into a journey TransitionMap/StepSpec with zero casts.
- Infer TNavItem from the navigation array (descriptor & { navigation?:
readonly TNavItem[] }), defaulting to NavigationItem only when absent, so
function-form `to: (ctx) => ...` type-checks with no generics while the
inferred-narrow item stays assignable to a NavigationItem-typed register().
Item 4 — derive step ordering + progress from the transition graph:
- resolveStepSequence(definition, options?) walks the static defineTransition
targets graph and returns the ordered step list (linear, or branch-selected).
- JourneyDefinition.steps: per-step { path, progressLabel } metadata, keyed and
type-checked against the real modules/entries (single source of truth).
- useJourneyProgress on React and Vue returns { index, total, label, steps } —
the stepCount that JourneyHost (item 2) deferred, now graph-derived.
Adds acceptance + unit + type tests across the touched packages; updates the
changelog and marks items 3 & 4 shipped in the tracker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KThnW7wGDMovXu8JdV4q2E
…dback-items-9jen4y
Follow-up to the item 3/4 PR, addressing code-review findings: - resolveStepSequence: match the `branch` resolver's return against the fork's declared `targets` by module+entry, so a ref that isn't a real target (or `undefined`) stops the walk instead of being followed blindly. - resolveStepSequence docs: note that `wildcard` transitions are not walked, and that the start step is derived by invoking `initialState`/`start` (which must be safe to call with the provided input). - useJourneyProgress (React + Vue): correct the `total` JSDoc — it is the best-effort statically-resolved spine length (partial on forks/unannotated/ maxSteps), always >= 1 for a derivable start, `null` only for an empty sequence — not the previously-documented "null when no instance / unannotated". Document that `index` (live) can reach/exceed `total` (static), and that `options.sequence` should be referentially stable. - define-module type-test comments: the function-form `to` fix infers `TNavItem` from the `navigation` array; it does not change the default to `NavigationItemBase`. Corrected the misleading comments. - Add a test for the branch-resolver-returns-foreign-ref case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqsgpXv75wiufKKkjTNLXz
… resolveStepSequence Address CodeRabbit review on PR #83. `resolveStepSequence(definition)` previously type-checked for journeys whose `initialState` needs an input, then called `initialState(undefined)` when `input` was omitted. Model the options so a non-void `TInput` requires either `input` (handed to the factories) or `start` (naming the first step, skipping them), while void-input journeys keep fully-optional options. The trailing options argument becomes required for non-void input via `StepSequenceOptionsArg`, so `resolveStepSequence(def)` is now a compile error precisely when it would have called `initialState(undefined)`. Propagate the same input-awareness through `useJourneyProgress` (React and Vue): `UseJourneyProgressOptions.sequence` is required (with `input`/`start`) for non-void journeys, keeping the forwarding to `resolveStepSequence` sound. Add type-level regression tests proving the bare non-void call, empty options, walk-only options, and wrong-typed `input` are all rejected, while void-input journeys and `input`/`start` forms are accepted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTynWaEhJz9FYF95seoPvH
Address CodeRabbit nit on PR #83: "Requires annotated (defineTransition) handlers" → "Requires handlers annotated with defineTransition". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTynWaEhJz9FYF95seoPvH
…inferred Address CodeRabbit outside-diff review on PR #83. With partial explicit generics, `defineModule<AppDeps, AppSlots>(descriptor)` locked `TNavItem` to its `NavigationItem` default (TypeScript can't partially infer a call's type arguments — spelling some forces the rest to their defaults), so function-form `to: (ctx) => ...` stopped type-checking unless the caller also spelled the full nav-item generic. That partial form is exactly what the scaffolder templates and core READMEs shipped. Add a curried overload — `defineModule<AppDeps, AppSlots>()(descriptor)` — to all five cores (frontend-core + the four router cores). The first, empty call pins the app-wide TSharedDependencies / TSlots (and optional TMeta); the second infers TNavItem + TDescriptor from the descriptor, so a typed shell fixes deps/slots while function-form `to` stays inferred. Same idiom as defineJourney. The direct `defineModule(descriptor)` (zero generics) and fully-explicit four-generic forms are unchanged. - Type tests: curried function-form/plain-string `to` in frontend-core; curried literal-preservation in tanstack-router-core; curried `ctx` narrowing in vue-core. - Migrate scaffolder templates (react-router / tanstack / vue), the five core READMEs, the getting-started / shell-patterns / framework-mode / remote- capability docs, and the example modules to the curried form; refresh CLI scaffolding snapshots. Full-repo typecheck (147/147) and the five core + three CLI test suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTynWaEhJz9FYF95seoPvH
Documentation/examples/CLI review pass:
- README: correct the per-family version claims in Project status
(react-router-modules & tanstack-react-modules: core/runtime v2.x,
cli/testing v3.x; @modular-react core/react v2.x, testing v1.x), and drop
the stale "some examples declare library deps with semver ranges" clause —
every example pins `workspace:*`.
- docs/remote-capability-manifests.md: retarget five broken source links from
the non-existent `packages/core/src/{types,remote-manifest}.ts` to their real
home in `packages/frontend-core/src/`.
- docs/navigation.md link fix: promote "Journey-contributed nav" in the
journeys README to a heading so the existing `#journey-contributed-nav`
anchor resolves.
- examples/README: add the undocumented examples to the index tree and
descriptions — integration-setup-journey, journey-invoke (RR + TSR),
tanstack remote-capabilities, and the catalog demo portal.
- CLI: genericize the shared `create store` help text (was hardcoded
"Zustand", wrong for the Vue CLI whose store is `createStore`); add
@modular-vue/cli to cli-core's consumer list (README + package.json); add
the now-required `scaffold` field to cli-core's preset example; document the
`serve` subcommand in the catalog README's CLI section; refresh the catalog
package status marker (v0.2 → v1.x).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UTynWaEhJz9FYF95seoPvH
Follow-up usefulness pass on the documentation review: - README: add the experimental Angular family (@angular-router-modules/core, @modular-angular/angular) to Project status and a new "Angular (experimental)" Packages subsection — marked v0.1.x / core-only and explicitly not yet a peer of the router integrations, mirroring how the Nuxt integration is noted. - getting-started (React Router, TanStack Router): name the shipped journey features the journeys section previously glossed as "the runtime hooks" — <JourneyHost>/useJourneyHost, useJourneyProgress, and useJourneySync. - getting-started (Vue Router): note the matching Vue journey composables in the closing further-reading list. Verified every referenced export exists in @modular-react/journeys and @modular-vue/journeys. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UTynWaEhJz9FYF95seoPvH
…cts-83-gjee83 # Conflicts: # README.md # docs/remote-capability-manifests.md
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (65)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the next two items from docs/consumer-feedback-production-app.md.
Item 3 — defineModule usable by real apps (all five variants:
@modular-frontend/core + the four router cores):
entryPoints/exitPoints keep their literal keys instead of widening. typeof
someModule now drops into a journey TransitionMap/StepSpec with zero casts.
readonly TNavItem[] }), defaulting to NavigationItem only when absent, so
function-form
to: (ctx) => ...type-checks with no generics while theinferred-narrow item stays assignable to a NavigationItem-typed register().
Item 4 — derive step ordering + progress from the transition graph:
targets graph and returns the ordered step list (linear, or branch-selected).
type-checked against the real modules/entries (single source of truth).
the stepCount that JourneyHost (item 2) deferred, now graph-derived.
Adds acceptance + unit + type tests across the touched packages; updates the
changelog and marks items 3 & 4 shipped in the tracker.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01KThnW7wGDMovXu8JdV4q2E