Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 29 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces a "Remote Capability Manifests" pattern for modular apps, enabling backend-delivered JSON to drive frontend slots and navigation. It adds core library exports ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Shell
participant Store
participant Module
participant Client
participant Backend
User->>Shell: selectProject(id)
activate Shell
Shell->>Store: selectProject(id)
activate Store
Store->>Store: status = loading
Store->>Client: fetchManifest(id)
Client->>Backend: GET /projects/{id}.json
Backend-->>Client: RemoteManifest JSON
Client->>Client: validateManifest()
Client-->>Store: validated manifest
Store->>Store: activeManifest = manifest<br/>status = ready
deactivate Store
Store-->>Module: subscription triggered
activate Module
Module->>Module: dynamicSlots = activeManifest.slots
Module->>Shell: recalculateSlots()
deactivate Module
Shell->>Shell: re-render UI
deactivate Shell
Shell-->>User: updated integration UI
sequenceDiagram
participant Shell
participant Module
participant Client
participant Backend
participant Store
Shell->>Shell: resolveRegistry()
activate Module
Module->>Module: onRegister()
Module->>Client: fetchManifests()
Client->>Backend: GET /integrations.json
Backend-->>Client: RemoteManifest[] JSON
Client->>Client: validateManifests()
Client-->>Module: validated manifests[]
deactivate Module
Module->>Store: setManifests(manifests)
activate Store
Store->>Store: status = ready<br/>manifests = [...]
deactivate Store
Store-->>Shell: subscription triggered
Shell->>Shell: recalculateSlots()
Shell->>Module: evaluateDynamicSlots()
Module->>Module: mergeRemoteManifests(manifests)
Module-->>Shell: merged slots
Shell->>Shell: re-render UI with<br/>all integrations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
| export interface RemoteModuleManifest< | ||
| TSlots extends SlotMapOf<TSlots> = SlotMap, | ||
| TNavItem extends NavigationItemBase = RemoteNavigationItem, | ||
| > { | ||
| readonly id: string; | ||
| readonly version: string; | ||
| readonly slots?: { readonly [K in keyof TSlots]?: TSlots[K] }; | ||
| readonly navigation?: readonly TNavItem[]; | ||
| readonly meta?: Readonly<Record<string, unknown>>; | ||
| } |
There was a problem hiding this comment.
How our JSON could fit into this different manifest shape?
Do we need to have a mapping step?
Just for the context, this is the current shape of our JSON from the RFC we've been planning for:
{
"authentication": {
"type": "oauth"
},
"filters": [
{
"id": "search",
"type": "search",
"query": "contains(name, '{value}')"
},
{
"id": "last-modified",
"type": "daterange",
"query": "lastModifiedAt ge {value.from} and lastModifiedAt le {value.to}"
}
],
"capabilities": {
"importTracking": {
"version": 1,
"data": { "pollingIntervalMs": 5000 }
}
}
}
There was a problem hiding this comment.
I would imagine you would define Integration as a slot, and then put the shape of this manifest into its definition. Slots exists exactly for this purpose, to aggregate X entries of the same shell-defined interface.
There was a problem hiding this comment.
let me update both docs and example to make it clearer
| export function mergeRemoteManifests< | ||
| TSlots extends SlotMapOf<TSlots> = SlotMap, | ||
| TNavItem extends NavigationItemBase = RemoteNavigationItem, | ||
| >( | ||
| manifests: readonly RemoteModuleManifest<TSlots, TNavItem>[], | ||
| ): MergedRemoteManifests<TSlots, TNavItem> { |
There was a problem hiding this comment.
Why would we need to merge different manifests?
Wouldn't one just for the active integration be enough?
If user at any point would switch projects that'd use different integration we could just discard the prior manifest. We still wouldn't re-fetch manifests every time we need to render anything on the UI, but just once whenever project is being open (and when project is being set up to support different auth flows).
What do you think?
There was a problem hiding this comment.
it is already possible. I've expanded examples to showcase this scenario as well
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
examples/react-router/remote-capabilities/shell/package.json (1)
2-2: Use a workspace-unique package name.The workspace currently has no duplicate package names, but
"shell"is a generic name that provides little context within the monorepo. Consider using a scoped name such as@example/remote-capabilities-shellfor better clarity and consistency with other workspace packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/shell/package.json` at line 2, The package name "shell" in package.json is too generic; change the "name" field to a workspace-unique, scoped identifier (for example "@example/remote-capabilities-shell") to match monorepo conventions and avoid collisions—update the value of the "name" property in the same package.json so other packages reference this new scoped name consistently.examples/react-router/remote-capabilities/shell/src/services/integrations-client.ts (2)
40-47: Consider a timeout / AbortController onfetch.Not a blocker for an example, but since this client is pitched as "real-looking" and is explicitly suggested as a template to swap for a real endpoint in production, an unbounded
fetchcan hang module registration indefinitely if the backend stalls. A shortAbortSignal.timeout(...)demonstrates the realistic pattern readers should copy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/shell/src/services/integrations-client.ts` around lines 40 - 47, The fetchManifests function currently performs an unbounded fetch; add a timeout using an AbortSignal (e.g., AbortSignal.timeout or an AbortController with a setTimeout) and pass the signal into fetch(endpoint, { signal }) in fetchManifests to avoid hanging registration; catch abort/timeout errors and rethrow a clear Error (e.g., "Fetch manifests timed out") or include the original error, and ensure any timers/controllers are cleaned up; reference the fetchManifests function and the endpoint variable when making the change.
7-30: Validation logic is duplicated with the swap example.This near-identical
validateManifest(s)hand-roll also lives inexamples/react-router/active-project-manifest/shell/src/services/integrations-client.ts. Since both examples live in the same monorepo, extracting a sharedvalidateRemoteManifest(raw)into@modular-react/core(or a small example-shared util) would both DRY the code and — more importantly — make the validation story part of the library surface the docs already promote. Acceptable to leave as-is if the intent is for each example to stand alone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/shell/src/services/integrations-client.ts` around lines 7 - 30, The validateManifests function is duplicated across examples (see validateManifest/validateManifests); extract this runtime validator into a shared utility (e.g., export a validateRemoteManifest(raw): readonly AppRemoteManifest[] from `@modular-react/core` or a repo-level example-shared util), then replace the local function in integrations-client.ts (and the near-identical one in active-project-manifest) with an import of validateRemoteManifest and remove the local implementation so both examples reuse the single exported validator.examples/react-router/remote-capabilities/shell/src/components/Sidebar.tsx (1)
8-19: Active-link check uses exact path equality.
location.pathname === hrefwon't highlight the "Integrations" entry when the user is on a nested route like/integrations/salesforce. If nested integration routes are ever added (the module already lazy-loadsIntegrationsPage), the sidebar will appear to deselect. Considerlocation.pathname === href || location.pathname.startsWith(href + "/"), or use<NavLink>fromreact-routerwhich handles this natively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/shell/src/components/Sidebar.tsx` around lines 8 - 19, The active-link check in linkStyle currently uses exact equality (location.pathname === href) which fails for nested routes; update the check inside linkStyle (the function named linkStyle) to consider nested paths by using a condition like location.pathname === href || location.pathname.startsWith(href + "/"), or replace the manual logic by using react-router's NavLink which provides an isActive prop and handles nested routes automatically; update usages of linkStyle or switch the Sidebar link elements to NavLink so "Integrations" remains highlighted on nested routes.examples/react-router/remote-capabilities/app-shared/package.json (1)
5-12:main/types/exportspoint at raw.tssources.Fine for an internal workspace package consumed via
link-workspace-packages, but worth noting: any external consumer (or a bundler without TS transform configured fornode_modules) won't be able to import this. Since this is an example-only package that is explicitly not published, no action required — just flagging for awareness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/app-shared/package.json` around lines 5 - 12, The package.json currently points "main", "types", and the "exports" targets at raw TypeScript sources ("./src/index.ts"), which breaks consumers or bundlers that don't transform node_modules; update these fields to point to compiled outputs (e.g., set "main" to the built JS like "./dist/index.js", "types" to the generated declarations like "./dist/index.d.ts", and update the "exports" import/types entries to the corresponding ./dist files) and ensure the build step produces those ./dist artifacts (or alternatively keep as-is but document that this package is internal/example-only and should not be consumed without a TS transform).examples/react-router/active-project-manifest/README.md (1)
44-44: Nit: add a language hint to the fenced block.markdownlint flags the file-tour block (MD040). Use
```text(or similar) to silence the warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/active-project-manifest/README.md` at line 44, The fenced code block used for the file-tour example in README.md lacks a language hint which triggers markdownlint MD040; update that fenced block (the triple-backtick block containing the file-tour output/examples) to include a language hint such as ```text (or ```bash/```console if more appropriate) so the block is explicitly annotated and the linter warning is silenced.examples/react-router/remote-capabilities/modules/integrations/src/index.ts (1)
35-54: Minor: no re-entrancy / staleness guard inonRegister.Fire-and-forget is fine for the example, but if
onRegisterruns more than once (test re-registration, HMR, or a future retry button), concurrentfetchManifests()resolutions could race and the older resolution would win. Consider either documenting this as single-shot, or tracking an in-flight token similar to the swap example'sactiveProjectIdstaleness check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/react-router/remote-capabilities/modules/integrations/src/index.ts` around lines 35 - 54, onRegister is fire-and-forget and can be re-entered, so concurrent fetchManifests() responses may race and stale results win; fix by introducing a staleness guard: before calling deps.integrationsClient.fetchManifests() generate a unique request token (e.g. a UUID or incrementing counter) and store it on the shared state (e.g. deps.integrations.currentFetchToken or integrations.fetchToken), then in both the .then and .catch handlers check that the token still matches before calling deps.integrations.setManifests, deps.integrations.setError, or deps.integrations.setStatus; only apply results when the token matches (older responses no-op). This keeps logic in onRegister and references the existing symbols onRegister, fetchManifests, setManifests, setError, and setStatus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 91-119: Add a step to run the examples' build scripts before
typechecking and testing so example-specific bundler/runtime regressions are
caught; in the "Build examples" job insert a step that runs the workspace build
for examples (e.g., run "pnpm turbo run build --filter=\"./examples/**\"")
placed after "Build library packages" and before "Typecheck examples" so example
packages like active-project-shell and remote-capabilities/shell get their vite
builds executed prior to typecheck/test.
In
`@examples/react-router/active-project-manifest/shell/src/components/ProjectPicker.tsx`:
- Around line 29-39: Currently clicking the already-active project always calls
selectProject and refetches; update the click handler in the projects.map button
so it no-ops when the project is active and status === "ready" but still allows
calls when the active project is in "error" or when switching projects.
Specifically, inside the onClick for the button rendered in projects.map, check
if project.id === activeProjectId and status === "ready" and return early;
otherwise call selectProject(project.id). Keep the existing isLoading disabled
behavior (isActive && status === "loading") so loading buttons remain disabled.
In
`@examples/react-router/active-project-manifest/shell/src/components/Sidebar.tsx`:
- Around line 36-64: The sidebar currently renders only navigation.groups and
will drop ungrouped entries and still render empty group headings; update the
rendering in Sidebar.tsx to first filter out groups with no visible items (use
group.items.filter(item => !item.hidden).length) and then render those groups,
and also render navigation.ungrouped (filtering out hidden items) alongside the
groups; keep using the existing Link and linkStyle(item.to) logic so ungrouped
entries appear and empty group headings are skipped.
In
`@examples/react-router/active-project-manifest/shell/src/services/integrations-client.ts`:
- Around line 32-33: The fetchManifest method interpolates projectId directly
into the request path causing malformed URLs when projectId contains characters
like "/", "?", "#", or spaces; update the fetch call inside fetchManifest to
URL-encode the projectId (use encodeURIComponent on the projectId argument
before building the `/projects/... .json` path) so the request always targets
the intended resource.
In `@examples/react-router/remote-capabilities/README.md`:
- Line 72: The README.md has a fenced code block starting with ``` that lacks a
language tag (markdownlint MD040); update that fenced block (the triple-backtick
block containing "remote-capabilities/") to include a language identifier such
as "text" by changing the opening fence to ```text so the block becomes a
labelled fenced code block.
In `@examples/react-router/remote-capabilities/shell/package.json`:
- Around line 15-18: The package.json dependencies for the example are using
published semver ranges instead of the workspace protocol, so replace the four
entries "@modular-react/core", "@modular-react/react",
"@react-router-modules/core", and "@react-router-modules/runtime" with workspace
references (e.g. "workspace:*") so PR-local workspace packages are resolved
(ensuring local exports like mergeRemoteManifests are available during the
example build).
---
Nitpick comments:
In `@examples/react-router/active-project-manifest/README.md`:
- Line 44: The fenced code block used for the file-tour example in README.md
lacks a language hint which triggers markdownlint MD040; update that fenced
block (the triple-backtick block containing the file-tour output/examples) to
include a language hint such as ```text (or ```bash/```console if more
appropriate) so the block is explicitly annotated and the linter warning is
silenced.
In `@examples/react-router/remote-capabilities/app-shared/package.json`:
- Around line 5-12: The package.json currently points "main", "types", and the
"exports" targets at raw TypeScript sources ("./src/index.ts"), which breaks
consumers or bundlers that don't transform node_modules; update these fields to
point to compiled outputs (e.g., set "main" to the built JS like
"./dist/index.js", "types" to the generated declarations like
"./dist/index.d.ts", and update the "exports" import/types entries to the
corresponding ./dist files) and ensure the build step produces those ./dist
artifacts (or alternatively keep as-is but document that this package is
internal/example-only and should not be consumed without a TS transform).
In `@examples/react-router/remote-capabilities/modules/integrations/src/index.ts`:
- Around line 35-54: onRegister is fire-and-forget and can be re-entered, so
concurrent fetchManifests() responses may race and stale results win; fix by
introducing a staleness guard: before calling
deps.integrationsClient.fetchManifests() generate a unique request token (e.g. a
UUID or incrementing counter) and store it on the shared state (e.g.
deps.integrations.currentFetchToken or integrations.fetchToken), then in both
the .then and .catch handlers check that the token still matches before calling
deps.integrations.setManifests, deps.integrations.setError, or
deps.integrations.setStatus; only apply results when the token matches (older
responses no-op). This keeps logic in onRegister and references the existing
symbols onRegister, fetchManifests, setManifests, setError, and setStatus.
In `@examples/react-router/remote-capabilities/shell/package.json`:
- Line 2: The package name "shell" in package.json is too generic; change the
"name" field to a workspace-unique, scoped identifier (for example
"@example/remote-capabilities-shell") to match monorepo conventions and avoid
collisions—update the value of the "name" property in the same package.json so
other packages reference this new scoped name consistently.
In `@examples/react-router/remote-capabilities/shell/src/components/Sidebar.tsx`:
- Around line 8-19: The active-link check in linkStyle currently uses exact
equality (location.pathname === href) which fails for nested routes; update the
check inside linkStyle (the function named linkStyle) to consider nested paths
by using a condition like location.pathname === href ||
location.pathname.startsWith(href + "/"), or replace the manual logic by using
react-router's NavLink which provides an isActive prop and handles nested routes
automatically; update usages of linkStyle or switch the Sidebar link elements to
NavLink so "Integrations" remains highlighted on nested routes.
In
`@examples/react-router/remote-capabilities/shell/src/services/integrations-client.ts`:
- Around line 40-47: The fetchManifests function currently performs an unbounded
fetch; add a timeout using an AbortSignal (e.g., AbortSignal.timeout or an
AbortController with a setTimeout) and pass the signal into fetch(endpoint, {
signal }) in fetchManifests to avoid hanging registration; catch abort/timeout
errors and rethrow a clear Error (e.g., "Fetch manifests timed out") or include
the original error, and ensure any timers/controllers are cleaned up; reference
the fetchManifests function and the endpoint variable when making the change.
- Around line 7-30: The validateManifests function is duplicated across examples
(see validateManifest/validateManifests); extract this runtime validator into a
shared utility (e.g., export a validateRemoteManifest(raw): readonly
AppRemoteManifest[] from `@modular-react/core` or a repo-level example-shared
util), then replace the local function in integrations-client.ts (and the
near-identical one in active-project-manifest) with an import of
validateRemoteManifest and remove the local implementation so both examples
reuse the single exported validator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24ff4164-d6e5-43db-892d-530cfc9eedb2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.github/workflows/ci.ymlREADME.mddocs/remote-capability-manifests.mdexamples/react-router/active-project-manifest/.gitignoreexamples/react-router/active-project-manifest/README.mdexamples/react-router/active-project-manifest/app-shared/package.jsonexamples/react-router/active-project-manifest/app-shared/src/index.tsexamples/react-router/active-project-manifest/app-shared/tsconfig.jsonexamples/react-router/active-project-manifest/modules/integrations/package.jsonexamples/react-router/active-project-manifest/modules/integrations/src/index.test.tsexamples/react-router/active-project-manifest/modules/integrations/src/index.tsexamples/react-router/active-project-manifest/modules/integrations/src/pages/IntegrationPage.tsxexamples/react-router/active-project-manifest/modules/integrations/tsconfig.jsonexamples/react-router/active-project-manifest/shell/index.htmlexamples/react-router/active-project-manifest/shell/package.jsonexamples/react-router/active-project-manifest/shell/public/projects/project-alpha.jsonexamples/react-router/active-project-manifest/shell/public/projects/project-beta.jsonexamples/react-router/active-project-manifest/shell/public/projects/project-gamma.jsonexamples/react-router/active-project-manifest/shell/src/components/Home.tsxexamples/react-router/active-project-manifest/shell/src/components/ProjectPicker.tsxexamples/react-router/active-project-manifest/shell/src/components/RootLayout.tsxexamples/react-router/active-project-manifest/shell/src/components/ShellLayout.tsxexamples/react-router/active-project-manifest/shell/src/components/Sidebar.tsxexamples/react-router/active-project-manifest/shell/src/main.tsxexamples/react-router/active-project-manifest/shell/src/projects.tsexamples/react-router/active-project-manifest/shell/src/services/integrations-client.tsexamples/react-router/active-project-manifest/shell/src/stores/integrations.tsexamples/react-router/active-project-manifest/shell/tsconfig.jsonexamples/react-router/active-project-manifest/shell/vite.config.tsexamples/react-router/active-project-manifest/tsconfig.base.jsonexamples/react-router/remote-capabilities/.gitignoreexamples/react-router/remote-capabilities/README.mdexamples/react-router/remote-capabilities/app-shared/package.jsonexamples/react-router/remote-capabilities/app-shared/src/index.tsexamples/react-router/remote-capabilities/app-shared/tsconfig.jsonexamples/react-router/remote-capabilities/modules/integrations/package.jsonexamples/react-router/remote-capabilities/modules/integrations/src/index.test.tsexamples/react-router/remote-capabilities/modules/integrations/src/index.tsexamples/react-router/remote-capabilities/modules/integrations/src/pages/IntegrationsPage.tsxexamples/react-router/remote-capabilities/modules/integrations/tsconfig.jsonexamples/react-router/remote-capabilities/shell/index.htmlexamples/react-router/remote-capabilities/shell/package.jsonexamples/react-router/remote-capabilities/shell/public/integrations.jsonexamples/react-router/remote-capabilities/shell/src/components/Home.tsxexamples/react-router/remote-capabilities/shell/src/components/RootLayout.tsxexamples/react-router/remote-capabilities/shell/src/components/ShellLayout.tsxexamples/react-router/remote-capabilities/shell/src/components/Sidebar.tsxexamples/react-router/remote-capabilities/shell/src/main.tsxexamples/react-router/remote-capabilities/shell/src/services/integrations-client.tsexamples/react-router/remote-capabilities/shell/src/stores/integrations.tsexamples/react-router/remote-capabilities/shell/tsconfig.jsonexamples/react-router/remote-capabilities/shell/vite.config.tsexamples/react-router/remote-capabilities/tsconfig.base.jsonpackages/core/src/index.tspackages/core/src/remote-manifest.test.tspackages/core/src/remote-manifest.tspnpm-workspace.yaml
| examples: | ||
| name: Build examples | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup Node | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 22.x | ||
| package-manager-cache: false | ||
|
|
||
| - name: Install pnpm | ||
| run: corepack enable && corepack prepare pnpm@latest --activate | ||
|
|
||
| - name: Install | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| # Library packages must be built first so the examples' typecheck | ||
| # and tests see emitted .d.ts files (the examples consume them via | ||
| # linked workspace packages). | ||
| - name: Build library packages | ||
| run: pnpm turbo run build --filter="./packages/*" | ||
|
|
||
| - name: Typecheck examples | ||
| run: pnpm turbo run typecheck --filter="./examples/**" | ||
|
|
||
| - name: Test examples | ||
| run: pnpm turbo run test --filter="./examples/**" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List example packages that define a build script.
# Expectation: Any listed packages should be covered by the examples CI job.
fd '^package\.json$' examples -0 \
| xargs -0 jq -r 'select(.scripts.build != null) | input_filename + " :: " + (.name // "(unnamed)") + " :: " + .scripts.build'Repository: kibertoad/modular-react
Length of output: 253
Add a step to build example packages in the CI job.
This job builds library packages, then typechecks/tests examples, but never runs the examples' own build scripts. At least two example packages define build scripts (active-project-shell and shell in remote-capabilities, both using vite), meaning bundler/runtime wiring regressions can pass CI.
Suggested adjustment
- name: Typecheck examples
run: pnpm turbo run typecheck --filter="./examples/**"
+ - name: Build examples
+ run: pnpm turbo run build --filter="./examples/**"
+
- name: Test examples
run: pnpm turbo run test --filter="./examples/**"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| examples: | |
| name: Build examples | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v6 | |
| - name: Setup Node | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: 22.x | |
| package-manager-cache: false | |
| - name: Install pnpm | |
| run: corepack enable && corepack prepare pnpm@latest --activate | |
| - name: Install | |
| run: pnpm install --frozen-lockfile | |
| # Library packages must be built first so the examples' typecheck | |
| # and tests see emitted .d.ts files (the examples consume them via | |
| # linked workspace packages). | |
| - name: Build library packages | |
| run: pnpm turbo run build --filter="./packages/*" | |
| - name: Typecheck examples | |
| run: pnpm turbo run typecheck --filter="./examples/**" | |
| - name: Test examples | |
| run: pnpm turbo run test --filter="./examples/**" | |
| examples: | |
| name: Build examples | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v6 | |
| - name: Setup Node | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: 22.x | |
| package-manager-cache: false | |
| - name: Install pnpm | |
| run: corepack enable && corepack prepare pnpm@latest --activate | |
| - name: Install | |
| run: pnpm install --frozen-lockfile | |
| # Library packages must be built first so the examples' typecheck | |
| # and tests see emitted .d.ts files (the examples consume them via | |
| # linked workspace packages). | |
| - name: Build library packages | |
| run: pnpm turbo run build --filter="./packages/*" | |
| - name: Typecheck examples | |
| run: pnpm turbo run typecheck --filter="./examples/**" | |
| - name: Build examples | |
| run: pnpm turbo run build --filter="./examples/**" | |
| - name: Test examples | |
| run: pnpm turbo run test --filter="./examples/**" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 91 - 119, Add a step to run the
examples' build scripts before typechecking and testing so example-specific
bundler/runtime regressions are caught; in the "Build examples" job insert a
step that runs the workspace build for examples (e.g., run "pnpm turbo run build
--filter=\"./examples/**\"") placed after "Build library packages" and before
"Typecheck examples" so example packages like active-project-shell and
remote-capabilities/shell get their vite builds executed prior to
typecheck/test.
| "@modular-react/core": "^1.0.0", | ||
| "@modular-react/react": "^1.0.0", | ||
| "@react-router-modules/core": "^2.0.0", | ||
| "@react-router-modules/runtime": "^2.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
target="examples/react-router/remote-capabilities/shell/package.json"
tmp="$(mktemp)"
fd package.json -E node_modules -E dist -E build |
while read -r file; do
jq -r --arg file "$file" 'select(.name != null) | [.name, $file] | `@tsv`' "$file"
done | sort > "$tmp"
jq -r '.dependencies // {} | to_entries[] | [.key, .value] | `@tsv`' "$target" |
while IFS=$'\t' read -r name range; do
if awk -F '\t' -v name="$name" '$1 == name { found=1 } END { exit !found }' "$tmp"; then
if [[ "$range" != workspace:* ]]; then
printf 'local dependency does not use workspace protocol: %s %s\n' "$name" "$range"
fi
fi
doneRepository: kibertoad/modular-react
Length of output: 392
Pin local packages through the workspace protocol.
The dependencies @modular-react/core, @modular-react/react, @react-router-modules/core, and @react-router-modules/runtime are workspace packages that must use the workspace:* protocol to ensure PR-local changes are resolved instead of published versions. Without this, the example build will miss PR-local exports like mergeRemoteManifests.
Suggested dependency range update
- "@modular-react/core": "^1.0.0",
- "@modular-react/react": "^1.0.0",
- "@react-router-modules/core": "^2.0.0",
- "@react-router-modules/runtime": "^2.0.0",
+ "@modular-react/core": "workspace:*",
+ "@modular-react/react": "workspace:*",
+ "@react-router-modules/core": "workspace:*",
+ "@react-router-modules/runtime": "workspace:*",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@modular-react/core": "^1.0.0", | |
| "@modular-react/react": "^1.0.0", | |
| "@react-router-modules/core": "^2.0.0", | |
| "@react-router-modules/runtime": "^2.0.0", | |
| "@modular-react/core": "workspace:*", | |
| "@modular-react/react": "workspace:*", | |
| "@react-router-modules/core": "workspace:*", | |
| "@react-router-modules/runtime": "workspace:*", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/react-router/remote-capabilities/shell/package.json` around lines 15
- 18, The package.json dependencies for the example are using published semver
ranges instead of the workspace protocol, so replace the four entries
"@modular-react/core", "@modular-react/react", "@react-router-modules/core", and
"@react-router-modules/runtime" with workspace references (e.g. "workspace:*")
so PR-local workspace packages are resolved (ensuring local exports like
mergeRemoteManifests are available during the example build).
Summary by CodeRabbit
New Features
Documentation
Chores