Introduce Path type alias for path arrays in LiveObjects code#2222
Conversation
WalkthroughThis PR introduces a shared ChangesPath Type Standardization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Slightly more readable, I think, especially for `string[][]`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d3e8b5b to
e454f57
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/plugins/liveobjects/liveobject.ts`:
- Line 7: Replace the runtime import of Path with a type-only import: since Path
is declared as a type (export type Path = string[]), update the import statement
that references Path in this module to use TypeScript's type-only import form so
the compiler/transpiler emits no runtime import for Path; locate the import line
importing Path from './path' in src/plugins/liveobjects/liveobject.ts and change
it accordingly so usages of the Path type in this file remain valid but no
runtime import is emitted.
🪄 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: 53a539b8-da93-4d81-adfe-2eed42528978
📒 Files selected for processing (4)
src/plugins/liveobjects/liveobject.tssrc/plugins/liveobjects/path.tssrc/plugins/liveobjects/pathobject.tssrc/plugins/liveobjects/pathobjectsubscriptionregister.ts
| import { ROOT_OBJECT_ID } from './constants'; | ||
| import { InstanceEvent } from './instance'; | ||
| import { ObjectData, ObjectMessage, ObjectOperation } from './objectmessage'; | ||
| import { Path } from './path'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Toolchain settings that affect type-only imports =="
fd -HI 'tsconfig*.json' -t f | while read -r f; do
echo "-- $f"
rg -n '"verbatimModuleSyntax"|"importsNotUsedAsValues"|"isolatedModules"' "$f" || true
done
fd -HI 'babel*.json|babel*.js|.swcrc|package.json' -t f | while read -r f; do
echo "-- $f"
rg -n '`@babel/preset-typescript`|onlyRemoveTypeImports|swc' "$f" || true
done
echo
echo "== Value imports of Path from ./path =="
rg -nP "import\s+\{\s*Path\s*\}\s+from\s+'\.\/path';" --type=ts
echo
echo "== Type-only imports of Path from ./path =="
rg -nP "import\s+type\s+\{\s*Path\s*\}\s+from\s+'\.\/path';" --type=ts || trueRepository: ably/ably-js
Length of output: 24586
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check Path export in path.ts ==="
cat -n "src/plugins/liveobjects/path.ts" | head -50
echo ""
echo "=== Check Path usage in liveobject.ts ==="
cat -n "src/plugins/liveobjects/liveobject.ts" | head -100
echo ""
echo "=== Check Path usage in pathobject.ts ==="
cat -n "src/plugins/liveobjects/pathobject.ts" | head -100
echo ""
echo "=== Check Path usage in pathobjectsubscriptionregister.ts ==="
cat -n "src/plugins/liveobjects/pathobjectsubscriptionregister.ts" | head -100
echo ""
echo "=== Check tsconfig.json for relevant settings ==="
cat "tsconfig.json" | grep -A 20 "compilerOptions"Repository: ably/ably-js
Length of output: 12892
🏁 Script executed:
#!/bin/bash
# Quick check to see how Path is actually used in the method bodies of liveobject.ts
# to confirm it's truly type-only and not used at runtime
rg -A 5 "getFullPaths" src/plugins/liveobjects/liveobject.ts | head -30Repository: ably/ably-js
Length of output: 412
Use a type-only import for Path.
Since Path is declared as export type Path = string[], import it with import type syntax to signal that it's a type-only symbol and avoid runtime import emission issues with stricter TypeScript configurations or transpilers.
Suggested patch
-import { Path } from './path';
+import type { Path } from './path';🤖 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/plugins/liveobjects/liveobject.ts` at line 7, Replace the runtime import
of Path with a type-only import: since Path is declared as a type (export type
Path = string[]), update the import statement that references Path in this
module to use TypeScript's type-only import form so the compiler/transpiler
emits no runtime import for Path; locate the import line importing Path from
'./path' in src/plugins/liveobjects/liveobject.ts and change it accordingly so
usages of the Path type in this file remain valid but no runtime import is
emitted.
Slightly more readable, I think, especially for
string[][].Summary by CodeRabbit