fix: decouple openchoreo-clent-node from Backstage dependencies#226
fix: decouple openchoreo-clent-node from Backstage dependencies#226kaviththiranga wants to merge 1 commit intoopenchoreo:mainfrom
Conversation
Extract a standalone Logger interface and move the Backstage-specific createOpenChoreoClientFromConfig factory into a dedicated backstage.ts module, so the core client can be consumed by non-Backstage projects (e.g. the VSCode extension). Backstage packages are now optional peer dependencies. Signed-off-by: Kavith Lokuhewage <kaviththiranga@gmail.com>
📝 WalkthroughWalkthroughThe changes decouple Backstage-specific dependencies from the core OpenChoreo client library by converting them to optional peer dependencies, extracting Backstage-related factory functions into a dedicated module, and introducing a standalone Logger interface to replace direct Backstage LoggerService dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/openchoreo-client-node/package.json`:
- Around line 36-43: Update the package README to document the optional peer
dependencies declared in package.json and explain which exports require them:
clearly state that `@backstage/config` (and any Backstage-only helpers) is an
optional peer dependency and that functions like
createOpenChoreoClientFromConfig require `@backstage/config` to be
installed/importable (import-time resolution will fail if it's not present); add
a short usage section showing alternative usage patterns (directly passing a
config object or using createOpenChoreoClientFromConfig with app-config.yaml +
`@backstage/config` installed), and list any other Backstage-specific symbols or
helpers and their dependency requirements so consumers know when they must
install those optional peers.
In `@packages/openchoreo-client-node/src/index.ts`:
- Around line 23-24: Remove the unconditional re-export of
createOpenChoreoClientFromConfig from the main entry (index.ts) to avoid
importing `@backstage/config` for non-Backstage consumers: delete the line
exporting createOpenChoreoClientFromConfig (the export that references
'./backstage') so only core factories from './factory' are exported from the
main module, and if Backstage consumers need the helper, instruct them to import
it directly from './backstage' or add a conditional export entry in package.json
to expose a separate “/backstage” entrypoint instead of re-exporting it from
index.ts.
🧹 Nitpick comments (1)
packages/openchoreo-client-node/package.json (1)
32-35: Consider tightening the peer dependency version constraints.The
>=1.0.0constraint is very permissive and may allow incompatible major versions. Consider using caret ranges like^1.0.0to allow minor/patch updates while preventing breaking changes from future major versions (e.g.,@backstage/configv2.x could have breaking API changes).Proposed version constraints
"peerDependencies": { - "@backstage/backend-plugin-api": ">=1.0.0", - "@backstage/config": ">=1.0.0" + "@backstage/backend-plugin-api": "^1.0.0", + "@backstage/config": "^1.0.0" },
| "peerDependenciesMeta": { | ||
| "@backstage/backend-plugin-api": { | ||
| "optional": true | ||
| }, | ||
| "@backstage/config": { | ||
| "optional": true | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which files import `@backstage` packages
rg -l "@backstage/" packages/openchoreo-client-node/src/Repository: openchoreo/backstage-plugins
Length of output: 165
🏁 Script executed:
# List the structure of the source directory
find packages/openchoreo-client-node/src -type f -name "*.ts" -o -name "*.js" | head -20Repository: openchoreo/backstage-plugins
Length of output: 883
🏁 Script executed:
# Check index.ts to see what's exported
cat -n packages/openchoreo-client-node/src/index.tsRepository: openchoreo/backstage-plugins
Length of output: 1973
🏁 Script executed:
# Check backstage.ts to see the implementation
cat -n packages/openchoreo-client-node/src/backstage.tsRepository: openchoreo/backstage-plugins
Length of output: 1811
🏁 Script executed:
# Check for README or documentation files
find packages/openchoreo-client-node -type f \( -name "README*" -o -name "*.md" \) | head -10Repository: openchoreo/backstage-plugins
Length of output: 113
🏁 Script executed:
# Check the package.json to understand the export configuration
cat -n packages/openchoreo-client-node/package.json | head -60Repository: openchoreo/backstage-plugins
Length of output: 1978
🏁 Script executed:
# Check the README for documentation about dependencies
cat -n packages/openchoreo-client-node/README.mdRepository: openchoreo/backstage-plugins
Length of output: 392
Update the package README to document optional peer dependencies and their usage.
The isolation of Backstage-specific code is well-implemented and properly documented in the code comments. However, the inline JSDoc documentation states that createOpenChoreoClientFromConfig expects configuration in app-config.yaml, yet consumers cannot import this function without @backstage/config installed—this will fail at import resolution time, not runtime. The package README should explicitly document which functions require optional peer dependencies and include usage guidance for Backstage-specific features.
🤖 Prompt for AI Agents
In `@packages/openchoreo-client-node/package.json` around lines 36 - 43, Update
the package README to document the optional peer dependencies declared in
package.json and explain which exports require them: clearly state that
`@backstage/config` (and any Backstage-only helpers) is an optional peer
dependency and that functions like createOpenChoreoClientFromConfig require
`@backstage/config` to be installed/importable (import-time resolution will fail
if it's not present); add a short usage section showing alternative usage
patterns (directly passing a config object or using
createOpenChoreoClientFromConfig with app-config.yaml + `@backstage/config`
installed), and list any other Backstage-specific symbols or helpers and their
dependency requirements so consumers know when they must install those optional
peers.
| // Export Backstage-specific factory (requires @backstage/config peer dependency) | ||
| export { createOpenChoreoClientFromConfig } from './backstage'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/openchoreo-client-node/package.json | jq '.exports // "no exports field"'Repository: openchoreo/backstage-plugins
Length of output: 90
🏁 Script executed:
fd "index.ts|backstage.ts" packages/openchoreo-client-node/src --type fRepository: openchoreo/backstage-plugins
Length of output: 368
🏁 Script executed:
cat -n packages/openchoreo-client-node/src/backstage.ts | head -30Repository: openchoreo/backstage-plugins
Length of output: 1136
🏁 Script executed:
cat packages/openchoreo-client-node/package.json | jq '.dependencies, .peerDependencies, .optionalDependencies'Repository: openchoreo/backstage-plugins
Length of output: 217
🏁 Script executed:
cat -n packages/openchoreo-client-node/src/index.tsRepository: openchoreo/backstage-plugins
Length of output: 1973
🏁 Script executed:
rg "createOpenChoreoApiClient|factory" packages/openchoreo-client-node/src --type ts -A 2 -B 2Repository: openchoreo/backstage-plugins
Length of output: 2988
Remove the re-export of createOpenChoreoClientFromConfig from the main entry point, or add conditional exports.
The re-export at line 24 defeats the intended isolation of the @backstage/config dependency. When any import occurs from the main entry point, Node.js evaluates backstage.ts, which unconditionally imports @backstage/config. Non-Backstage consumers without this peer dependency will encounter a MODULE_NOT_FOUND error at runtime, even if they never use this function.
Since the core factory functions are already re-exported from ./factory (lines 13-21), remove the createOpenChoreoClientFromConfig re-export from index.ts. Backstage consumers can import directly from the backstage module if needed:
import { createOpenChoreoClientFromConfig } from '@openchoreo/openchoreo-client-node/backstage';Alternatively, add an exports field to package.json to provide conditional entry points and protect non-Backstage consumers from loading the Backstage-specific code.
🤖 Prompt for AI Agents
In `@packages/openchoreo-client-node/src/index.ts` around lines 23 - 24, Remove
the unconditional re-export of createOpenChoreoClientFromConfig from the main
entry (index.ts) to avoid importing `@backstage/config` for non-Backstage
consumers: delete the line exporting createOpenChoreoClientFromConfig (the
export that references './backstage') so only core factories from './factory'
are exported from the main module, and if Backstage consumers need the helper,
instruct them to import it directly from './backstage' or add a conditional
export entry in package.json to expose a separate “/backstage” entrypoint
instead of re-exporting it from index.ts.
Extract a standalone Logger interface and move the Backstage-specific
createOpenChoreoClientFromConfig factory into a dedicated backstage.ts
module, so the core client can be consumed by non-Backstage projects
(e.g. the VSCode extension). Backstage packages are now optional peer
dependencies.
Summary by CodeRabbit