Skip to content

fix: declare eventemitter3 as SDK runtime dependency#32

Open
mojobeeping wants to merge 2 commits into
gladiaio:mainfrom
mojobeeping:mojobeep/eventemitter-runtime-dependency
Open

fix: declare eventemitter3 as SDK runtime dependency#32
mojobeeping wants to merge 2 commits into
gladiaio:mainfrom
mojobeeping:mojobeep/eventemitter-runtime-dependency

Conversation

@mojobeeping

@mojobeeping mojobeeping commented Jun 8, 2026

Copy link
Copy Markdown

Summary

  • moves eventemitter3 from peer dependencies to runtime dependencies for the published @gladiaio/sdk package
  • keeps ws and undici as optional peers
  • updates the Bun lock workspace metadata for the same dependency move

Fixes #30.

Reproduction

A clean Yarn 1 install of the published package currently warns about the unmet peer and then fails at SDK import time because dist/v2/live/session.js imports eventemitter3 internally:

tmp=$(mktemp -d)
cd "$tmp"
corepack yarn init -y >/dev/null 2>&1
corepack yarn add @gladiaio/sdk@1.0.4 --ignore-scripts --silent
node --input-type=module -e "import('@gladiaio/sdk')"

Observed before this patch:

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'eventemitter3' imported from .../node_modules/@gladiaio/sdk/dist/v2/live/session.js

Validation

  • npm run build --workspaces=false from packages/sdk-js passed
  • npm pack --ignore-scripts --json --workspaces=false from packages/sdk-js passed
  • clean Yarn 1 install of the patched tarball successfully imported @gladiaio/sdk
  • patched tarball package metadata includes dependencies.eventemitter3 = >=5
  • git diff --check passed
  • node -e "JSON.parse(require('fs').readFileSync('packages/sdk-js/package.json','utf8'))" passed

Note: ./node_modules/.bin/nx run sdk-js:test --skipNxCache currently runs but fails two unrelated src/client.test.ts assertions because those tests instantiate a Gladia API host without apiKey or proxy apiUrl. The dependency change is covered by the packed clean-install smoke above.

Summary by CodeRabbit

  • Chores
    • Updated SDK dependency declarations to include eventemitter3 as a direct dependency, removing the requirement for consumers to supply it separately.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mojobeeping, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 30 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 149f906d-5a6a-4243-9e15-30e754dbbceb

📥 Commits

Reviewing files that changed from the base of the PR and between 8a564c3 and a56e33e.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • packages/sdk-js/package.json
📝 Walkthrough

Walkthrough

The SDK package declares eventemitter3 as a direct dependency instead of a peer dependency. This change ensures that consumers automatically receive the required eventemitter3 package when they install the SDK, resolving the missing dependency error reported in issue #30.

Changes

Dependency Management

Layer / File(s) Summary
eventemitter3 direct dependency
packages/sdk-js/package.json
A dependencies section is added with eventemitter3 >=5, removing it from peerDependencies so consumers no longer need to supply it separately.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A package once lost in the peer haze,
Now nestled in dependencies' craze,
No more missing errors at runtime's call,
eventemitter3 is bundled for all! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving eventemitter3 from peerDependencies to dependencies, which is the core modification in the PR.
Linked Issues check ✅ Passed The PR directly addresses issue #30 by declaring eventemitter3 as a runtime dependency, ensuring it is installed with the SDK and resolving the import-time failure.
Out of Scope Changes check ✅ Passed All changes are narrowly scoped to moving eventemitter3 from peerDependencies to dependencies and updating lock file metadata; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/sdk-js/package.json (1)

32-32: Use a caret range for safer semver on eventemitter3

"eventemitter3": ">=5" would allow future breaking major updates (e.g., 6.x+). Since the latest published version is currently 5.0.4 and no GitHub security advisories were reported for eventemitter3, constraining to v5 is the safer pattern.

📦 Suggested semver range
   "dependencies": {
-    "eventemitter3": ">=5"
+    "eventemitter3": "^5.0.0"
   },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk-js/package.json` at line 32, Update the dependency declaration
for eventemitter3 in package.json to use a caret range that locks to major
version 5 (e.g., change "eventemitter3": ">=5" to "eventemitter3": "^5.0.0" or
"eventemitter3": "^5"); locate the "eventemitter3" entry in package.json and
replace the version string so you get compatible patch/minor updates within v5
but avoid automatically accepting a potential v6 breaking change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/sdk-js/package.json`:
- Line 32: Update the dependency declaration for eventemitter3 in package.json
to use a caret range that locks to major version 5 (e.g., change
"eventemitter3": ">=5" to "eventemitter3": "^5.0.0" or "eventemitter3": "^5");
locate the "eventemitter3" entry in package.json and replace the version string
so you get compatible patch/minor updates within v5 but avoid automatically
accepting a potential v6 breaking change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c47bc60-3d69-44d5-90f9-fad99fa57a16

📥 Commits

Reviewing files that changed from the base of the PR and between f0172f6 and 8a564c3.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • packages/sdk-js/package.json

@mojobeeping

Copy link
Copy Markdown
Author

Addressed the CodeRabbit dependency-range nit in commit a56e33e by changing eventemitter3 from >=5 to ^5.0.0 in packages/sdk-js/package.json and the matching bun.lock workspace metadata.

Validation repeated after the change:

  • node -e "JSON.parse(require('fs').readFileSync('packages/sdk-js/package.json','utf8'))"
  • git diff --check
  • npm run build --workspaces=false from packages/sdk-js
  • npm pack --ignore-scripts --json --workspaces=false from packages/sdk-js
  • Clean temp-project install from the packed tarball, followed by import('@gladiaio/sdk') and eventemitter3 resolution check

Note: the build still emits the repo's existing neutral-platform warnings for dynamic fs/path imports in src/v2/prerecorded/client.ts, but exits successfully.

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.

Missing dep eventemitter3

1 participant