Support Ably v2#50
Conversation
WalkthroughThis PR upgrades the laravel-echo codebase from Ably v1 to v2, converting all callback-based APIs to promise-based equivalents, updating the type system to reference top-level ChangesAbly v2 SDK upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@package.json`:
- Line 49: Update package.json's engines.node to match Ably v2's Node
requirement: change the "engines.node" value from ">=10" to ">=16" (or
">=16.0.0") so it aligns with the declared peerDependencies.ably "^2.0". Ensure
the updated engines.node entry is in package.json and run a quick install/test
to confirm no other engine constraints conflict.
In `@src/echo.ts`:
- Line 1: The current export incorrectly re-exports type-only imports as runtime
values; in src/echo.ts keep the existing import type { Channel, PresenceChannel
} and change the export to separate runtime and type exports—export { Connector
} (runtime) and export type { Channel, PresenceChannel } (types) so Channel and
PresenceChannel are only exported as types and not as values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 501ca499-dc1f-45d7-9eb3-5d2d7aaa6494
📒 Files selected for processing (26)
.github/README.mdCHANGELOG-ABLY.mdpackage.jsonrollup.config.jssrc/channel/ably-channel.tssrc/channel/ably-presence-channel.tssrc/channel/ably-private-channel.tssrc/channel/ably/auth.tssrc/channel/ably/utils.tssrc/channel/null-presence-channel.tssrc/channel/presence-channel.tssrc/channel/pusher-presence-channel.tssrc/channel/socketio-presence-channel.tssrc/connector/ably-connector.tssrc/connector/connector.tssrc/connector/null-connector.tssrc/echo.tstests/ably/ably-channel.test.tstests/ably/ably-connection.test.tstests/ably/ably-presence-channel.test.tstests/ably/ably-private-channel.test.tstests/ably/ably-sandbox.test.tstests/ably/ably-user-login.test.tstests/ably/ably-user-rest-publishing.test.tstests/ably/setup/mock-auth-server.tstypings/ably.ts
| "@typescript-eslint/eslint-plugin": "^5.14.0", | ||
| "@typescript-eslint/parser": "^5.14.0", | ||
| "ably": "^1.2", | ||
| "ably": "^2.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json, urllib.request
with open("package.json", "r", encoding="utf-8") as f:
pkg = json.load(f)
repo_engine = pkg.get("engines", {}).get("node")
ably_range = (
pkg.get("peerDependencies", {}).get("ably")
or pkg.get("devDependencies", {}).get("ably")
)
print("package.json engines.node:", repo_engine)
print("package.json ably range:", ably_range)
data = json.loads(
urllib.request.urlopen("https://registry.npmjs.org/ably", timeout=20).read().decode("utf-8")
)
versions = sorted(
[v for v in data.get("versions", {}) if v.startswith("2.")],
key=lambda s: [int(x) for x in s.split(".")]
)
print("\nRecent ably 2.x versions and their engines:")
for v in versions[-10:]:
eng = data["versions"][v].get("engines")
print(f"{v}: {eng}")
PYRepository: ably-forks/laravel-echo
Length of output: 2742
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba package.json | sed -n '35,90p'Repository: ably-forks/laravel-echo
Length of output: 110
🌐 Web query:
Ably JavaScript SDK v2 engines.node minimum Node version
💡 Result:
The Ably JavaScript SDK v2 requires a minimum Node.js version of 16 [1][2]. The engines.node field in the package.json for the SDK is set to >=16 [3]. While v2 was initially tested against Node.js versions 16, 18, and 20 [1][3], it remains compatible with Node.js 16 or greater [4].
Citations:
- 1: https://github.com/ably/ably-js/blob/HEAD/CHANGELOG.md
- 2: https://changelog.ably.com/js-client-library-release-v2-0-0-288796
- 3: [ECO-4121] Publicly document v2 platform compatibility and update CI accordingly ably/ably-js#1633
- 4: https://ably.com/docs/getting-started/node
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== engines block =="
rg -n '"engines"\s*:' package.json || true
awk 'NR>=1 && NR<=120 {print NR ":" $0}' package.json | rg -n --context 0 '"engines"|"node"|\"ably\"' -n || true
echo
echo "== excerpt around ably entries =="
awk 'NR>=40 && NR<=90 {print NR ":" $0}' package.json | rg -n '"ably"|\"engines\"|\"node\"' || true
echo
echo "== parsed values =="
node -e "const pkg=require('./package.json'); console.log('engines.node=',pkg.engines?.node); console.log('ably=', pkg.peerDependencies?.ably || pkg.devDependencies?.ably || pkg.dependencies?.ably);"Repository: ably-forks/laravel-echo
Length of output: 421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== engines.node excerpt =="
awk 'NR>=55 && NR<=75 {print NR ":" $0}' package.json
echo
echo "== ably entries =="
rg -n '"ably"\s*:' package.json
rg -n '"ably"\s*:' package.json -n --context 2 || trueRepository: ably-forks/laravel-echo
Length of output: 843
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Fallback: print a slightly wider window for peer/dev deps
echo "== deps excerpt (40-90) =="
awk 'NR>=40 && NR<=90 {print NR ":" $0}' package.jsonRepository: ably-forks/laravel-echo
Length of output: 1124
Align engines.node with Ably v2’s Node requirement
package.json sets engines.node to >=10, but it also declares peerDependencies.ably as ^2.0 (Ably JS v2), which requires Node.js >=16. Update engines.node accordingly to avoid consumer install/runtime breakage.
🤖 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 `@package.json` at line 49, Update package.json's engines.node to match Ably
v2's Node requirement: change the "engines.node" value from ">=10" to ">=16" (or
">=16.0.0") so it aligns with the declared peerDependencies.ably "^2.0". Ensure
the updated engines.node entry is in package.json and run a quick install/test
to confirm no other engine constraints conflict.
| @@ -1,4 +1,4 @@ | |||
| import { Channel, PresenceChannel } from './channel'; | |||
| import type { Channel, PresenceChannel } from './channel'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification for import-type/export mismatch in src/echo.ts
rg -nP "import\s+type\s+\{[^}]*\b(Channel|PresenceChannel)\b[^}]*\}\s+from\s+'\.\/channel';" src/echo.ts
rg -nP "export\s+\{[^}]*\bChannel\b[^}]*\};|export\s+\{[^}]*\bPresenceChannel\b[^}]*\};" src/echo.tsRepository: ably-forks/laravel-echo
Length of output: 180
Fix type-only re-exports in src/echo.ts to avoid TS “type used as a value” build failures.
Channel/PresenceChannel are imported via import type, but are re-exported as runtime values in export { Connector, Channel, PresenceChannel };. Re-export them as types instead.
Suggested fix
-import type { Channel, PresenceChannel } from './channel';
+import type { Channel, PresenceChannel } from './channel';
import { AblyConnector, Connector, PusherConnector, SocketIoConnector, NullConnector } from './connector';
@@
-export { Connector, Channel, PresenceChannel };
+export { Connector };
+export type { Channel, PresenceChannel };🤖 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 `@src/echo.ts` at line 1, The current export incorrectly re-exports type-only
imports as runtime values; in src/echo.ts keep the existing import type {
Channel, PresenceChannel } and change the export to separate runtime and type
exports—export { Connector } (runtime) and export type { Channel,
PresenceChannel } (types) so Channel and PresenceChannel are only exported as
types and not as values.
ttypic
left a comment
ttypic
left a comment
There was a problem hiding this comment.
Thanks a lot for the contribution! We’ll take a closer look next week. Do you have any other reason for the upgrade besides keeping the dependency up to date?
|
Yes, allow a stricter CSP configuration. Ably v1’s webpack bundle uses new Function("return this") for global object detection, which is blocked by strict script-src CSPs (without unsafe-eval). Upgrading to v2 eliminates that eval call. |
2 participants
Updates the Ably integration to require and work with ably@^2.0, including v2 type exports, promise-based realtime/presence/auth calls, and removal of ably/promises usage. Preserves the Echo channel API while handling Ably v2 clientId-change reconnects and presence operation failures correctly. Updates sandbox-backed tests to use the nonprod sandbox endpoint and documents the breaking dependency change in the Ably changelog.
Summary by CodeRabbit
Breaking Changes
Improvements