Skip to content

Conversation

addaleax
Copy link
Collaborator

@addaleax addaleax commented Oct 3, 2025

Make the shell-bson and shell-api packages more runtime-independent (as they are supposed to be) by using util and crypto as progressive enhancement enablers rather than required packages.

This specifically makes shell-bson fully usable in a bare-bones JS environment.

Make the `shell-bson` and `shell-api` packages more runtime-independent
(as they are supposed to be) by using `util` and `crypto` as progressive
enhancement enablers rather than required packages.

This specifically makes `shell-bson` fully usable in a bare-bones JS
environment.
@addaleax addaleax requested a review from a team as a code owner October 3, 2025 13:26
@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 13:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR reduces runtime dependencies for the shell-bson and shell-api packages by making util and crypto modules optional progressive enhancements rather than required imports. This allows these packages to work in bare-bones JavaScript environments.

Key changes include:

  • Replacing direct crypto usage with BSON library UUID generation
  • Implementing lazy loading of util.inspect with fallback functionality
  • Converting promisify usage to native Promise constructor

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/shell-bson/src/shell-bson.ts Replaces crypto-based UUID generation with bson.UUID()
packages/shell-bson/src/printable-bson.ts Adds lazy loading for util.inspect with fallback for typed array inspection
packages/shell-bson/src/bson-export.ts Adds UUID type imports and exports
packages/shell-api/src/shell-api.ts Replaces promisify with native Promise constructor in sleep method
packages/shell-api/src/runtime-independence.spec.ts Updates test to remove crypto and util from allowed Node builtins
packages/shell-api/src/helpers.ts Implements lazy loading for crypto and util.inspect with appropriate fallbacks
packages/service-provider-node-driver/src/node-driver-service-provider.ts Adds UUID to BSON library exports

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@addaleax addaleax added the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Oct 3, 2025
Copy link
Contributor

@lerouxb lerouxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although it does feel like ideally these should be pluggable so someone could still make something like inspect work the way they want if they care about it. Rather than it be either hardcoded as the common-js-imported module OR the hardcoded fallback.

@addaleax
Copy link
Collaborator Author

addaleax commented Oct 3, 2025

@lerouxb Yeah, absolutely makes sense. For the crypto bit I've done this here, don't actually use it but you're right that it should be pluggable.

For the util.inspect ones, I think we'd need a better story there overall in order to make it pluggable, since the current uses of it are not just tied to the API but also to the Node.js implementation of that API

@addaleax addaleax merged commit 847965c into main Oct 3, 2025
137 of 144 checks passed
@addaleax addaleax deleted the runtime-independence-improvements branch October 3, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants