Skip to content

Conversation

arthurbleilzup
Copy link
Contributor

No description provided.

Signed-off-by: Arthur Bleil <[email protected]>
@gitbotzup
Copy link

PR Summary

This pull request focuses on improving resource management, compatibility, and performance across the backend, client, and shared packages. The main highlights include robust disposal logic to prevent memory leaks, improved compatibility with Node.js environments by replacing import assertions, and performance optimizations such as HTML caching. Several package versions are also bumped, and new tarballs are added.


Changed Files and Their Summaries

Backend

  • packages/backend/package.json

    • Bumped the package version from 0.8.2 to 0.8.5. No other changes.
  • packages/backend/rollup.config.mjs

    • Replaced import assertions for JSON with Node.js createRequire to load package.json. Improves compatibility with Node.js environments that do not support import assertions.
  • packages/backend/src/MessageHandler.ts

    • Added robust disposal logic: includes a disposed flag, queue size limit (MAX_QUEUE_SIZE), and proper cleanup of listeners and pending promises.
    • Now supports a disposable message listener and ensures no messages are sent or processed after disposal.
    • Cleans up streaming map and handles queue overflow by dropping the oldest message and logging a warning.
  • packages/backend/src/VSCodeViewProvider.ts

    • Added an isResolving flag to prevent concurrent calls to resolveWebviewView.
    • Disposes of the previous bridge before creating a new one.
    • Logs a warning if resolveWebviewView is called while already resolving.
    • Ensures proper cleanup on view disposal.
  • packages/backend/src/VSCodeWebview.ts

    • Introduced a static htmlCache to cache HTML content by key, improving performance by avoiding redundant HTML loading and processing.
    • Set retainContextWhenHidden to true in webview options.
  • packages/backend/src/VSCodeWebviewBridge.ts

    • Simplified listenToMessagesFromClient to directly return the disposable from webview.onDidReceiveMessage, improving resource management and compatibility with new disposal logic.
  • packages/backend/stack-spot-vscode-async-webview-backend-0.8.5.tgz

    • Added a new tarball for the backend package version 0.8.5.
  • packages/backend/tsconfig.json

    • Cleaned up commented-out TypeScript compiler options for additional checks. No functional changes.

Client

  • packages/client/package.json

    • Bumped the package version from 0.7.4 to 0.7.6. No other changes.
  • packages/client/rollup.config.mjs

    • Replaced import assertions for JSON with Node.js createRequire to load package.json, mirroring the backend change for compatibility.
  • packages/client/src/VSCodeWeb.ts

    • Added robust disposal logic: includes a disposed flag, static instance tracking, and cleanup of event listeners, streams, and bridge calls.
    • Added automatic disposal on window unload.
    • Improved bridge proxy to cache method functions.
    • Added static methods to get and dispose the singleton instance.
    • These changes improve resource management and prevent memory leaks.
  • packages/client/stack-spot-vscode-async-webview-client-0.7.6.tgz

    • Added a new tarball for the client package version 0.7.6.

React

  • packages/react/rollup.config.mjs
    • Replaced import assertions for JSON with Node.js createRequire to load package.json.
    • Updated ESLint comment to also allow anonymous default exports.

Shared

  • packages/shared/rollup.config.mjs
    • Replaced import assertions for JSON with Node.js createRequire to load package.json, mirroring changes in other rollup configs.

Root

  • pnpm-lock.yaml
    • Lockfile updated, likely to reflect new package versions and dependencies.

Security Advice & Points of Attention

  • Resource Management:
    The new disposal logic in MessageHandler.ts and VSCodeWeb.ts significantly reduces the risk of memory leaks. However, it is still crucial that dispose() is called appropriately. If not, resources may still leak. Reviewers should ensure that all code paths that create these objects also dispose of them correctly.

  • Compatibility:
    The switch from import assertions to createRequire improves compatibility with more Node.js environments. Reviewers should verify that this change does not break any existing build or runtime workflows, especially in environments that previously relied on import assertions.


In summary:
This PR brings important improvements to resource management, compatibility, and performance. The most critical point for reviewers is to ensure that the new disposal logic is correctly integrated and that all resources are properly cleaned up to avoid memory leaks. No other significant security concerns were identified.
This is an AI-generated summary, which may be innacurate.
This aims only to assist human reviewers, and does not replace code reviews in any way.
Use responsibly and please submit any feedback to this form.

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