Conversation
Reviewer's GuideThis PR reorganizes the library’s exports and type definitions to clearly separate host- and client-side modules, consolidates shared types into dedicated folders, and updates package.json and build configs accordingly. Entity relationship diagram for shared types and their usageerDiagram
LOGGER ||--o| LOGMESSAGE : logs
LOGMESSAGE }|..|| LOGLEVEL : has
HOSTCALLS ||--o| LOGMESSAGE : can log
CLIENTCALLS ||--o| LOGMESSAGE : can log
STATE_REDUCER ||--o| PATCHES : uses
ACTION ||--o| STATE_REDUCER : triggers
WEBVIEWKEY ||--o| ACTION : identifies
WEBVIEWKEY ||--o| PATCH : identifies
Class diagram for new logging types (ILogger, LogMessage, LogLevel)classDiagram
class ILogger {
+debug(message: string, data?: Record<string, unknown>): void
+info(message: string, data?: Record<string, unknown>): void
+warn(message: string, data?: Record<string, unknown>): void
+error(message: string, data?: Record<string, unknown>): void
+dispose(): void
}
class LogMessage {
+type: 'log'
+level: LogLevel
+message: string
+data?: Record<string, unknown>
}
class LogLevel {
<<enum>>
DEBUG
INFO
WARN
ERROR
}
LogMessage --> LogLevel
WebviewLogger ..|> ILogger
Logger ..|> ILogger
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe PR restructures public exports and types, removes the index barrel and root export, introduces centralized logging/types modules, relocates/rehomes several type definitions, adds two type guards/utilities, and updates build entries and TypeScript type routing. Multiple imports are repointed to new modules without changing runtime behavior. Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant architectural refactoring to separate the library's host-side and client-side components into distinct, modular export entry points. This change aims to enhance code organization, clarify dependencies, and optimize bundle sizes by allowing consumers to import only the necessary parts. The refactoring also includes a comprehensive reorganization and centralization of various types and utility functions into more logical file structures, improving overall maintainability and clarity.
Highlights
- Modular Exports: The library now provides distinct entry points for host and client components, allowing consumers to import specific parts rather than a single, combined bundle. This is reflected in the updated
package.jsonexports. - Type Centralization: Common types, including logging interfaces (
ILogger,LogLevel) and RPC-related definitions, have been consolidated into a newsrc/lib/typesdirectory for improved organization and reusability across host and client modules. - Refactored IPC Reducer Logic: IPC reducer-related types and utility functions have been reorganized. Core reducer types are now in
src/lib/types/reducer.ts, while specific helper functions likeisFnKeyandisMyActionMessagehave been moved to dedicated client-side (src/lib/client/ipcReducer.ts) and host-side (src/lib/host/utils.ts) utility files respectively.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You’ve removed the default “.” export and main/module entries in package.json—either add back a root export under exports or confirm that all consumers now import exclusively from “./host” and “./client”.
- The commented-out
WebviewContextValueexport in src/lib/client.ts looks unintentional; either remove it entirely or restore it if it’s still needed by consumers. - In vite.config.ts the
indexentry was commented out—if you still need to build a combined bundle, re-add it or update any downstream scripts that expect anindexbuild.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You’ve removed the default “.” export and main/module entries in package.json—either add back a root export under exports or confirm that all consumers now import exclusively from “./host” and “./client”.
- The commented-out `WebviewContextValue` export in src/lib/client.ts looks unintentional; either remove it entirely or restore it if it’s still needed by consumers.
- In vite.config.ts the `index` entry was commented out—if you still need to build a combined bundle, re-add it or update any downstream scripts that expect an `index` build.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PR Code Suggestions ✨No code suggestions found for the PR. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 8e75d3e in 2 minutes and 34 seconds. Click for details.
- Reviewed
525lines of code in22files - Skipped
0files when reviewing. - Skipped posting
10draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .claude/settings.local.json:9
- Draft comment:
Add a newline at end of file for consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While having a newline at end of file is a common convention, this is a very minor stylistic issue. It would likely be caught by linters or formatters if it was important to the project. The file is a new JSON configuration file and JSON doesn't strictly require trailing newlines. The comment is technically correct and follows a common best practice. Missing newlines can cause issues with some tools. However, this is an extremely minor stylistic issue that would be better handled by automated tooling rather than manual review comments. It doesn't impact functionality. Delete this comment as it's too minor of an issue to warrant a manual review comment. This kind of feedback should be handled by linters/formatters.
2. package.json:16
- Draft comment:
Verify that separate host/client exports are correctly consumed by downstream projects. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue in the code.
3. src/lib/client.ts:1
- Draft comment:
Remove or document the commented-out export: WebviewContextValue. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Commented-out code can be a code smell and cleaning it up is generally good practice. However, this is a very minor issue. The comment doesn't indicate any functional problems, and there might be a temporary reason for keeping this commented code that we're not aware of without more context. The comment is technically correct as a code quality suggestion, but it's a very minor issue that doesn't affect functionality. We might be missing context about why the code was commented out temporarily. While it's a valid code quality suggestion, it's not important enough to warrant a PR comment. The author likely has a reason for commenting it out rather than deleting it. This comment should be removed as it's too minor of an issue and doesn't affect functionality. It violates the rule about not making purely informative comments.
4. src/lib/client/WebviewLogger.ts:1
- Draft comment:
Ensure the updated import paths reflect the new module organization. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the author to ensure something, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.
5. src/lib/client/ipcReducer.ts:3
- Draft comment:
The isFnKey utility is well implemented; no changes needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. src/lib/client/types.ts:30
- Draft comment:
DeferredPromise implementation is solid; ensure timeout management works during component unmount. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. src/lib/client/useVscodeState.ts:87
- Draft comment:
The action proxy robustly filters dangerous keys; consider adding tests for edge cases. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. src/lib/host/BaseWebviewViewProvider.ts:83
- Draft comment:
Using 'satisfies Patch' ensures type safety for patch messages; this is a good practice. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
9. src/lib/types/reducer.ts:1
- Draft comment:
Renaming from ipcReducer to reducer improves clarity; ensure all import paths are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment suggests a code improvement by renaming a variable for clarity, which is a valid suggestion. However, it also includes a request to ensure all import paths are updated, which violates the rule against asking the author to ensure something. The first part of the comment is useful, but the second part is not allowed.
10. vite.config.ts:17
- Draft comment:
Host and client entry points are clearly separated; confirm that external dependencies (e.g. 'vscode') remain external. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm something about external dependencies, which is against the rules. It doesn't provide a specific code suggestion or point out a specific issue with the code.
Workflow ID: wflow_Oz7tlkJ3poBtxNQM
You can customize 
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring to separate host and client code into distinct entry points. The reorganization of types into structured modules and the relocation of shared utilities greatly improve the project's structure and maintainability. The changes are consistent and align with the stated goals. I have one suggestion to improve the readability and robustness of the new isMyActionMessage type guard.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
22-33: Root import removed: treat as a breaking change and communicate it.Consumers must switch from
react-vscode-webview-ipcto subpathsreact-vscode-webview-ipc/hostand/client. Bump version accordingly and add a CHANGELOG/README note.- "version": "0.1.0", + "version": "0.2.0",Optionally add a README “Migration” section and release notes describing the removal of the root entrypoint.
src/lib/host/WebviewApiProvider.ts (1)
32-85: Async pruning bug: failed views are never removed.You push into
failedViewsinside async.thenrejection handlers, but prune synchronously right after the loop—before rejections run. Move pruning into the rejection/catch path and delete immediately.- // Track views that fail to receive messages - const failedViews: string[] = []; - // Send to all connected views for (const [viewId, connectedView] of this.connectedViews.entries()) { // eslint-disable-next-line sonarjs/no-try-promise try { - // Wrap postMessage in try-catch to handle synchronous exceptions - const postPromise = connectedView.view.webview.postMessage(event); - - // Handle async failures - postPromise.then( - () => { - // Message sent successfully - }, - (error: unknown) => { - this.logger.error( - `Failed to send event ${String(key)} to view ${connectedView.context.viewType}:${viewId}: ${getErrorMessage(error)}` - ); - - // Mark view for removal - failedViews.push(viewId); - } - ); + // Wrap postMessage; prune on async failure + void connectedView.view.webview.postMessage(event).catch((error: unknown) => { + this.logger.error( + `Failed to send event ${String(key)} to view ${connectedView.context.viewType}:${viewId}: ${getErrorMessage(error)}` + ); + this.logger.warn( + `Removing failed webview ${connectedView.context.viewType}:${viewId} from connectedViews` + ); + this.connectedViews.delete(viewId); + this.logger.info( + `Removed 1 failed webview from connectedViews. Remaining: ${this.connectedViews.size}` + ); + }); } catch (error) { // Handle synchronous exceptions from postMessage - this.logger.error( - `Exception while sending event ${String(key)} to view ${connectedView.context.viewType}:${viewId}: ${String(error)}` - ); - - // Mark view for removal - failedViews.push(viewId); + this.logger.error( + `Exception while sending event ${String(key)} to view ${connectedView.context.viewType}:${viewId}: ${getErrorMessage(error)}` + ); + this.logger.warn( + `Removing failed webview ${connectedView.context.viewType}:${viewId} from connectedViews` + ); + this.connectedViews.delete(viewId); } } - - // Prune failed views after iteration to avoid modifying collection during iteration - if (failedViews.length > 0) { - for (const viewId of failedViews) { - const connectedView = this.connectedViews.get(viewId as WebviewKey); - if (connectedView) { - this.logger.warn( - `Removing failed webview ${connectedView.context.viewType}:${viewId} from connectedViews` - ); - - // Only remove from connected views - let webviews handle their own disposal lifecycle - this.connectedViews.delete(viewId as WebviewKey); - } - } - - this.logger.info( - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Removed ${failedViews.length} failed webview(s) from connectedViews. Remaining: ${this.connectedViews.size}` - ); - }
🧹 Nitpick comments (16)
.claude/settings.local.json (1)
3-5: Tighten allowed commands to least privilege.Limit to the single build script you actually use instead of a wildcard.
- "Bash(npm run build:*)" + "Bash(npm run build)"package.json (2)
53-56: Peer deps look fine; confirm both subpaths truly require React.If the host entry is framework-agnostic, consider scoping React peer to the client subpath only (docs), or marking as optional via peerDependencyMeta.
57-92: Depcheck warnings: prune or ignore test-only deps.Either remove unused devDeps or teach depcheck to ignore them to keep CI green.
Option A — remove if genuinely unused:
- "@testing-library/dom": "^10.4.1", - "@testing-library/jest-dom": "^6.8.0", - "@testing-library/react": "^16.3.0", - "@testing-library/user-event": "^14.6.1", - "@vscode/test-cli": "^0.0.11", - "@vscode/test-electron": "^2.5.2", - "eslint-import-resolver-typescript": "^4.4.4", - "eslint-plugin-storybook": "^9.1.6",Option B — configure depcheck ignore list:
"scripts": { "preview": "vite preview" }, + "depcheck": { + "ignoreMatches": [ + "@testing-library/*", + "@vscode/test-*", + "eslint-import-resolver-typescript", + "eslint-plugin-storybook" + ] + },src/lib/client/ipcReducer.ts (1)
3-10: Type guard is solid; tighten the types to avoid casts.Use PropertyKey and a dictionary bound so you don’t need
as keyof Tto index.-export function isFnKey<T extends object>( - prop: string | symbol | number, - obj: T -): prop is FnKeys<T> { +export function isFnKey<T extends Record<PropertyKey, unknown>>( + prop: PropertyKey, + obj: T +): prop is FnKeys<T> { return ( - Object.prototype.hasOwnProperty.call(obj, prop) && typeof obj[prop as keyof T] === 'function' + Object.prototype.hasOwnProperty.call(obj, prop) && + typeof (obj as Record<PropertyKey, unknown>)[prop] === 'function' ); }src/lib/types/log.ts (1)
28-41: HardenisLogMessagevalidation (range-check level, validate data when present).Prevents false positives and noisy logs from malformed payloads.
export function isLogMessage(value: unknown): value is LogMessage { if (value === null || typeof value !== 'object' || Array.isArray(value)) { return false; } if (!('type' in value) || value.type !== 'log') { return false; } - if (!('level' in value) || typeof value.level !== 'number') { + if (!('level' in value) || typeof value.level !== 'number') { return false; } if (!('message' in value) || typeof value.message !== 'string') { return false; } - return true; + // Narrow level to known enum range + const lvl = (value as { level: number }).level; + if (lvl < LogLevel.DEBUG || lvl > LogLevel.ERROR) { + return false; + } + // If data is present, it must be a non-null object + if ('data' in value && (value as { data?: unknown }).data !== undefined) { + const d = (value as { data?: unknown }).data; + if (d === null || typeof d !== 'object' || Array.isArray(d)) { + return false; + } + } + return true; }src/lib/host/WebviewApiProvider.ts (1)
67-78: Avoid brand erasure and casts.If you keep a failed list, type it as
WebviewKey[]to preserve branding and removeas WebviewKeycasts. Superseded by the immediate-delete refactor above.src/lib/client/WebviewProvider.tsx (1)
32-36: Drop unused genericHto simplify JSX usage and inference.
Hisn’t inferred from props and only annotates a call; keeping it can make JSX generics brittle.-export const WebviewProvider = <T extends ClientCalls, H extends HostCalls>({ +export const WebviewProvider = <T extends ClientCalls>({ children, viewType, contextKey, }: WebviewProviderProps<T>) => { @@ - } else if (isViewApiEvent<H>(message)) { + } else if (isViewApiEvent(message)) {Also applies to: 139-151
src/lib/client/WebviewLogger.ts (1)
1-2: Split mixed import to satisfy lint rule and avoid side‑effect imports.-import { LogLevel, type ILogger, type LogMessage } from '../types'; +import { LogLevel } from '../types'; +import type { ILogger, LogMessage } from '../types'; import type { VsCodeApi } from './types';src/lib/client/useVscodeState.ts (2)
8-21: Strengthen patch guard: require string keysAlign with host guard and structured-clone limitations by asserting key is a string.
'key' in message && 'patch' in message && + typeof (message as Record<string, unknown>).key === 'string' && message.type === PATCH &&
80-90: Block symbol actions in the actorPrevent creating actions with symbol keys; they won’t serialize and will be rejected by the updated guards.
- get(_, prop) { - if (typeof prop !== 'string' && typeof prop !== 'symbol') { + get(_, prop) { + if (typeof prop !== 'string') { throw new TypeError(`Invalid action type: ${String(prop)}`); } - if (typeof prop === 'string' && dangerousKeys.has(prop)) { + if (dangerousKeys.has(prop)) { throw new Error(`Dangerous action key is blocked: ${prop}`); }src/lib/client/types.ts (2)
51-55: Fix VsCodeApi.postMessage return type for webview sideIn the webview context, postMessage returns void. Using Thenable can confuse consumers.
export interface VsCodeApi { - postMessage(message: unknown): Thenable<boolean>; + postMessage(message: unknown): void; getState(): unknown; setState(state: unknown): void; }If you intended to unify with vscode.Webview.postMessage (host side) which is Thenable, clarify the duality in docs or split the types.
57-59: Constrain StateReducer to object-shaped action mapsPrevents misuse and improves inference.
-export type StateReducer<S, A> = { +export type StateReducer<S, A extends object> = { [Key in FnKeys<A>]: (prevState: S, patch: Patches<A>[Key]) => S; };src/lib/host/BaseWebviewViewProvider.ts (1)
49-74: Harden action handling with try/catchA throwing delegate can propagate and destabilize the host. Catch, log, and consider posting an error response or skipping the patch.
- const messageListener = webviewView.webview.onDidReceiveMessage(async (message) => { + const messageListener = webviewView.webview.onDidReceiveMessage(async (message) => { if (isLogMessage(message)) { this.handleLogMessage(message); return; } if (isMyActionMessage<A>(message, this.providerId)) { this.logger.debug('Received action message from webview', { message }); - - const delegateFn = this.webviewActionDelegate[message.key]; - if (typeof delegateFn !== 'function') { - throw new TypeError(`Unknown action key: ${String(message.key)}`); - } - - const patch = await delegateFn(...message.params); - - this._view?.webview.postMessage({ - type: PATCH, - providerId: this.providerId, - key: message.key, - patch, - }); + try { + const delegateFn = this.webviewActionDelegate[message.key]; + if (typeof delegateFn !== 'function') { + throw new TypeError(`Unknown action key: ${String(message.key)}`); + } + const patch = await delegateFn(...message.params); + this._view?.webview.postMessage({ + type: PATCH, + providerId: this.providerId, + key: message.key, + patch, + }); + } catch (err) { + this.logger.error('Action handler failed', { err, key: String(message.key) }); + } return; }src/lib/types/reducer.ts (1)
7-11: Limit FnKeys to string keys and bound T to objectPrevents symbol keys from flowing into Action/Patch and aligns with messaging constraints.
-export type FnKeys<T> = { +export type FnKeys<T extends object> = Extract<{ // eslint-disable-next-line @typescript-eslint/no-explicit-any [K in keyof T]: T[K] extends (...args: any[]) => any ? K : never; -}[keyof T]; +}[keyof T], string>;src/lib/host.ts (1)
4-6: Breaking surface change: confirm SemVer and release notesRemoving prior exports (e.g., HostCalls/WebviewLayout) and slimming types is a breaking API for consumers. Bump major and document migration in the changelog.
src/lib/client.ts (1)
7-7: HostCalls in the client barrel may blur host/client boundaries.If the goal is separation, consider keeping HostCalls in a host/shared entrypoint instead of client.
-export type { ClientCalls, HostCalls, ViewApiResponse, ViewApiError } from './types'; +export type { ClientCalls, ViewApiResponse, ViewApiError } from './types';Outside this file (example):
// src/lib/host.ts export type { HostCalls } from './types';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.claude/settings.local.json(1 hunks)package.json(1 hunks)src/lib/client.ts(1 hunks)src/lib/client/WebviewContext.tsx(1 hunks)src/lib/client/WebviewLogger.ts(1 hunks)src/lib/client/WebviewProvider.tsx(1 hunks)src/lib/client/ipcReducer.ts(1 hunks)src/lib/client/types.ts(2 hunks)src/lib/client/useLogger.ts(1 hunks)src/lib/client/useVscodeState.ts(1 hunks)src/lib/host.ts(1 hunks)src/lib/host/BaseWebviewViewProvider.ts(1 hunks)src/lib/host/ILogger.ts(0 hunks)src/lib/host/WebviewApiProvider.ts(1 hunks)src/lib/host/logger.ts(1 hunks)src/lib/host/utils.ts(1 hunks)src/lib/index.ts(0 hunks)src/lib/types/index.ts(1 hunks)src/lib/types/log.ts(1 hunks)src/lib/types/reducer.ts(2 hunks)src/lib/types/rpc.ts(0 hunks)vite.config.ts(0 hunks)
💤 Files with no reviewable changes (4)
- vite.config.ts
- src/lib/index.ts
- src/lib/host/ILogger.ts
- src/lib/types/rpc.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/lib/host/utils.ts (1)
src/lib/types/reducer.ts (3)
WebviewKey(3-3)Action(17-22)ACT(5-5)
src/lib/client/ipcReducer.ts (1)
src/lib/types/reducer.ts (1)
FnKeys(7-10)
src/lib/client/types.ts (2)
src/lib/client.ts (2)
WebviewLayout(9-9)StateReducer(9-9)src/lib/types/reducer.ts (2)
FnKeys(7-10)Patches(36-43)
src/lib/types/log.ts (1)
src/lib/host.ts (1)
ILogger(5-5)
src/lib/types/reducer.ts (1)
src/lib/host.ts (1)
ActionDelegate(6-6)
🪛 GitHub Check: Lint, Type Check, and Test
src/lib/client/WebviewProvider.tsx
[failure] 13-13:
TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime. Convert this to a top-level type qualifier to properly remove the entire import
🪛 GitHub Actions: CI
src/lib/client/WebviewProvider.tsx
[error] 13-13: ESLint: @typescript-eslint/no-import-type-side-effects - TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime. Convert this to a top-level type qualifier to properly remove the entire import.
src/lib/host/BaseWebviewViewProvider.ts
[error] 1-1: Missing dependencies: 'vscode' required by ./src/lib/host/BaseWebviewViewProvider.ts.
package.json
[warning] 1-1: Depcheck reported unused devDependencies: @testing-library/dom, @testing-library/jest-dom, @testing-library/react, @testing-library/user-event, @vscode/test-cli, @vscode/test-electron, eslint-import-resolver-typescript, eslint-plugin-storybook.
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (8)
package.json (1)
16-21: typesVersions mapping looks correct — build produced no d.ts; manual verification required.Build output in the sandbox showed "Missing d.ts outputs" and "sh: 1: tsc: not found", so the import-resolution smoke test couldn't run. Confirm locally that
npm run buildemitsdist/host.d.tsanddist/client.d.ts, then run the smoke test with TypeScript available (e.g., create the temp tsconfig and runnpx tsc -p <tmp> --noEmit) to verify imports fromreact-vscode-webview-ipc/hostand/clientresolve..claude/settings.local.json (1)
1-9: Confirmed: .claude/settings.local.json will not be published.
package.json has "files": ["dist"] andnpm pack --dry-runlisted only README.md and package.json (total files: 2); .claude/settings.local.json is present in the repo but was not included in the packed tarball.src/lib/types/index.ts (1)
1-4: Types barrel looks good.Re-exports and
Brandalias are clear and cohesive.src/lib/client/useLogger.ts (1)
3-5: LGTM.Type-only imports and module boundaries are aligned with the new layout.
src/lib/client/WebviewContext.tsx (1)
1-2: Import split LGTMMoving VsCodeApi to client-local types while keeping Host/Client calls from ../types is consistent with the new surface split. No behavioral change.
src/lib/types/reducer.ts (1)
24-29: ActionDelegate move looks goodCentralizing ActionDelegate here simplifies host/client import paths. No issues spotted.
src/lib/client.ts (2)
6-6: Restore or deprecate exports: isViewApiResponse / isViewApiEventRemoving these re-exports is a breaking change for consumers — either re-export them from src/lib/client.ts for one release (mark with @deprecated) or document the migration.
Defined: src/lib/types/rpc.ts (isViewApiResponse at ~line 87, isViewApiEvent at ~line 121). Used: src/lib/client/WebviewProvider.tsx (imports at top; runtime checks at ~lines 121 and 139).
Suggested one-line fix (non-breaking): add to src/lib/client.ts:
export { isViewApiResponse, isViewApiEvent } from './types';
8-9: No runtime exports —export typeis correct.WebviewKey is a type alias (src/lib/types/reducer.ts); WebviewLayout and StateReducer are type aliases (src/lib/client/types.ts). No change required.
| @@ -1,17 +1,15 @@ | |||
| import * as vscode from 'vscode'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
CI failing: add ‘vscode’ as a dependency and externalize for bundling
The build reports “Missing dependencies: 'vscode'…”. Add vscode as a devDependency (and optionally peerDependency) and ensure bundlers mark it external.
Minimal package.json patch:
{
"name": "react-vscode-webview-ipc",
"version": "…",
+ "peerDependencies": {
+ "vscode": "*"
+ },
"devDependencies": {
+ "vscode": "*"
}
}Also configure your bundler (tsup/rollup/esbuild) to externalize "vscode" (e.g., tsup: { external: ['vscode'] }).
🏁 Script executed:
#!/bin/bash
# Verify presence and usage
rg -n "from 'vscode'" -g 'src/**'
jq '.peerDependencies, .devDependencies' package.jsonLength of output: 1415
CI failing — add 'vscode' dev/peer dependency and externalize in bundler
Build reports "Missing dependencies: 'vscode'". Add "vscode" to package.json (devDependency and peerDependency) and mark it external in your bundler. Affected files importing vscode: src/lib/host/logger.ts, src/lib/host/WebviewApiProvider.ts, src/lib/host/BaseWebviewViewProvider.ts.
Minimal package.json patch:
{
"name": "react-vscode-webview-ipc",
"version": "…",
+ "peerDependencies": {
+ "vscode": "*"
+ },
"devDependencies": {
+ "vscode": "*"
}
}Also configure your bundler (tsup/rollup/esbuild) to externalize "vscode" (e.g., tsup: { external: ['vscode'] }).
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Missing dependencies: 'vscode' required by ./src/lib/host/BaseWebviewViewProvider.ts.
1 participant
PR Type
Enhancement
Description
Separate host and client exports into dedicated entry points
Reorganize type definitions into structured modules
Move shared utilities to appropriate locations
Remove main index.ts export file
Diagram Walkthrough
File Walkthrough
19 files
Update client exports and type importsMove types to shared locationExtract utility function for clientAdd client-specific type definitionsUpdate import paths for reorganizationUpdate imports for type reorganizationUpdate host exports and type importsUpdate imports for reorganized typesRemove file, moved to typesUpdate import path for typesUpdate import path for ILoggerExtract utility function for hostRemove main index export fileCreate types module indexMove logging types to dedicated moduleReorganize reducer types and utilitiesMove RPC types to dedicated moduleUpdate import paths for typesUpdate import paths for reorganization3 files
Remove main index entry pointAdd Claude configuration fileUpdate exports for separate entry pointsImportant
Separate host and client exports by restructuring
package.json, updatingvite.config.ts, and modifying TypeScript imports/exports.package.jsonby defining distinct paths forhostandclientinexportsandtypesVersions.vite.config.tsto build separatehostandcliententries.ILoggerandLogLevelfromhost/ILogger.tstotypes/log.ts.isLogMessageandLogMessagefromhost/WebviewLogger.tstotypes/log.ts.isMyActionMessagefromtypes/ipcReducer.tstohost/utils.ts.StateReducerandWebviewLayoutfromtypes/ipcReducer.tstoclient/types.ts.FnKeysandPatchesfromtypes/ipcReducer.tstotypes/reducer.ts.src/lib/index.tsandsrc/lib/host/ILogger.ts.src/lib/types/ipcReducer.tstosrc/lib/types/reducer.ts.src/lib/host/WebviewLogger.tstosrc/lib/client/WebviewLogger.ts.src/lib/host/utils.tsfor utility functions.src/lib/types/log.tsfor logging types.src/lib/types/index.tsto export types fromlog.tsandrpc.ts.This description was created by
for 8e75d3e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Breaking Changes
New Features
Refactor
Chores