refactor: rename @hs/federation-sdk to @rocket.chat/federation-sdk #211
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR renames packages from the @hs/* namespace to @rocket.chat/*, updates TypeScript path mappings, and migrates all import paths accordingly. It also adjusts federation-sdk packaging (main to bundle.js, exports map, files list), updates dependencies, and adds public re-exports in federation-sdk/src/index.ts including a crypto helper. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (75)
Comment |
sampaiodiego
force-pushed
the
rename-sdk-package
branch
from
aacda24 to
600817f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #211 +/- ##
=======================================
Coverage 80.99% 80.99%
=======================================
Files 63 63
Lines 4693 4693
=======================================
Hits 3801 3801
Misses 892 892 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/homeserver/src/controllers/federation/send-join.controller.ts (1)
2-2: Optional: co-locate EventID via SDK re‑export to reduce direct @hs/room dependency.If the SDK re‑exports EventID, you can simplify imports:
-import type { EventID } from '@hs/room'; -import { SendJoinService } from '@rocket.chat/federation-sdk'; +import { SendJoinService, type EventID } from '@rocket.chat/federation-sdk';packages/homeserver/src/controllers/internal/invite.controller.ts (1)
13-13: Remove unused _inviteService resolution.
It’s never used; keep DI clean.- const _inviteService = container.resolve(InviteService); + // InviteService not used here; remove resolution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
packages/federation-sdk/package.json(2 hunks)packages/federation-sdk/src/index.ts(1 hunks)packages/homeserver/package.json(1 hunks)packages/homeserver/src/controllers/federation/invite.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/profiles.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/rooms.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/send-join.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/state.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/transactions.controller.ts(1 hunks)packages/homeserver/src/controllers/federation/versions.controller.ts(1 hunks)packages/homeserver/src/controllers/internal/direct-message.controller.ts(1 hunks)packages/homeserver/src/controllers/internal/invite.controller.ts(1 hunks)packages/homeserver/src/controllers/internal/message.controller.ts(1 hunks)packages/homeserver/src/controllers/internal/room.controller.ts(1 hunks)packages/homeserver/src/controllers/key/server.controller.ts(1 hunks)packages/homeserver/src/controllers/well-known/well-known.controller.ts(1 hunks)packages/homeserver/src/homeserver.module.ts(1 hunks)packages/homeserver/src/middlewares/acl.middleware.ts(1 hunks)tsconfig.base.json(1 hunks)
🔇 Additional comments (20)
packages/federation-sdk/src/index.ts (1)
19-31: Good addition: public type re-exports.Re-exporting core/room types here improves DX for consumers importing from the SDK root.
packages/federation-sdk/package.json (2)
28-28: Publishing surface reduced to “dist” only.This is fine as long as any runtime assets (schemas, migrations, etc.) are emitted to dist. If not, broaden the files list or emit those assets during build.
2-2: Package rename aligns with import changes.Name change is consistent with the repo-wide migration.
packages/homeserver/src/controllers/federation/profiles.controller.ts (1)
2-2: Import scope update is correct.ProfilesService now references the renamed package; no functional changes.
packages/homeserver/src/controllers/federation/rooms.controller.ts (1)
1-1: Import scope update is correct.StateService now comes from the new package scope; OK.
packages/homeserver/src/controllers/internal/direct-message.controller.ts (1)
1-1: Import scope update is correct.RoomService import updated to the new namespace; no behavior change.
packages/homeserver/src/controllers/well-known/well-known.controller.ts (1)
1-1: Import scope update is correct.WellKnownService import aligns with the SDK rename.
packages/homeserver/src/controllers/federation/versions.controller.ts (1)
1-1: Import scope update is correct.ConfigService import migrated cleanly.
tsconfig.base.json (1)
46-48: Path alias rename verified — no remaining @hs referencesRepo-wide search found zero occurrences of "@hs/federation-sdk"; "@rocket.chat/federation-sdk" is referenced across packages and tsconfig.base.json maps "@rocket.chat/federation-sdk/" → "packages/federation-sdk/src/".
packages/homeserver/src/controllers/internal/room.controller.ts (1)
6-10: Import path rename looks correct.DI resolution should remain intact; no behavioral change.
packages/homeserver/src/controllers/federation/send-join.controller.ts (1)
2-2: Import path updated correctly.No logic changes; endpoint behavior unaffected.
packages/homeserver/src/controllers/internal/message.controller.ts (1)
2-2: Import path rename applied correctly.Service resolution via tsyringe remains consistent.
packages/homeserver/src/controllers/federation/transactions.controller.ts (1)
1-1: Imports updated as expected.Aligned with package rename; no runtime impact.
Also applies to: 6-6
packages/homeserver/src/homeserver.module.ts (1)
11-11: Module import switch to new SDK is correct.Types and factory import line up with the SDK rename.
packages/homeserver/src/controllers/key/server.controller.ts (1)
1-1: ServerService import path updated correctly.No functional changes.
packages/homeserver/src/controllers/federation/state.controller.ts (1)
2-2: EventService import path updated correctly.No changes to route behavior.
packages/homeserver/package.json (1)
23-23: Dependency migrated to @rocket.chat/federation-sdk — verifiedNo lingering "@hs/federation-sdk" references found; new package present at packages/federation-sdk/package.json and referenced in packages/homeserver/package.json (line 23). Consider publishing a deprecation stub for the old package to ease external consumers.
packages/homeserver/src/controllers/federation/invite.controller.ts (1)
1-1: LGTM on import rename.
No behavioral changes; aligns with the package move.packages/homeserver/src/middlewares/acl.middleware.ts (1)
2-3: LGTM — exports verified
errCodes and EventAuthorizationService are exported from packages/federation-sdk/src/index.ts (lines 55, 84).packages/homeserver/src/controllers/internal/invite.controller.ts (1)
2-2: Confirm tsyringe registrations updated to use new @rocket.chat/federation-sdk exportsrg shows no remaining "@hs/federation-sdk" imports and many files now import "@rocket.chat/federation-sdk". Script errors prevented verifying DI registrations — inspect container.register/registerSingleton/registerInstance and @inject/@Injectable usages (start with packages/homeserver/src/homeserver.module.ts) to ensure injected tokens/classes (InviteService, StateService, etc.) match the new exports.
| "main": "./dist/bundle.js", | ||
| "types": "./dist/index.d.ts", |
There was a problem hiding this comment.
Probable breakage: “main”/exports point to dist/bundle.js but build doesn’t produce it.
Your build script runs only “tsc”, which won’t emit bundle.js. This will break runtime resolution for consumers importing the package root. Also, mapping both “import” and “require” to the same JS file can break CJS if the file is ESM-only.
Recommend scoping this PR to the rename and keep entrypoints stable:
Option A (revert to non-bundled entrypoints; minimal change):
- "main": "./dist/bundle.js",
+ "main": "./dist/index.js",
"types": "./dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
- "import": "./dist/bundle.js",
- "require": "./dist/bundle.js"
+ "import": "./dist/index.js"
+ // Optionally add a CJS build later and point "require" to "./dist/index.cjs"
}
},Option B (if you really want a bundle): add a bundler step (tsup/rollup/esbuild) that emits both ESM and CJS and wire exports accordingly. Example (tsup):
- scripts:
- "build": "tsc --build --force",
+ "build": "tsup src/index.ts --format esm,cjs --dts --sourcemap --out-dir dist --clean"- exports:
- "import": "./dist/bundle.js",
- "require": "./dist/bundle.js"
+ "import": "./dist/index.mjs",
+ "require": "./dist/index.cjs"Given the PR objective (rename only), Option A is safer.
Also applies to: 10-11
🤖 Prompt for AI Agents
In packages/federation-sdk/package.json around lines 5-6 (and also affecting
lines 10-11), the package.json points "main" to ./dist/bundle.js which the
current build (tsc) does not produce, breaking consumers; revert to safe,
minimal change by pointing entrypoints to the files tsc actually emits (e.g.,
set "main" to ./dist/index.js and "types" to ./dist/index.d.ts and adjust
"module"/"exports" entries similarly), or if you intend to ship a bundle
instead, add a bundler step (tsup/rollup/esbuild) to the build script and update
package.json exports to provide both ESM and CJS outputs—pick one approach and
make package.json and build scripts consistent so runtime resolution matches
emitted artifacts.
836c3a0 to
4095db0
Compare
Summary by CodeRabbit