feat: Removing /output wrapper project from CLI package.json template#111
Open
szanata wants to merge 4 commits into
Open
feat: Removing /output wrapper project from CLI package.json template#111szanata wants to merge 4 commits into
szanata wants to merge 4 commits into
Conversation
szanata
marked this pull request as draft
April 7, 2026 20:23
szanata
force-pushed
the
chore/remove_output_wrapper
branch
from
June 9, 2026 22:36
d1d5094 to
5139dc4
Compare
szanata
force-pushed
the
chore/remove_output_wrapper
branch
from
June 29, 2026 17:39
5139dc4 to
8601ea2
Compare
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
szanata
force-pushed
the
chore/remove_output_wrapper
branch
from
June 29, 2026 17:43
8601ea2 to
9f7b159
Compare
szanata
marked this pull request as ready for review
June 29, 2026 19:29
catsby
reviewed
Jun 30, 2026
|
|
||
| return packageJsonDeps; | ||
| } catch ( error ) { | ||
| debug( 'Failed to read local package.json: %O', error ); |
Collaborator
There was a problem hiding this comment.
I think this conflates a corrupt or unreadable file (invalid/broken structure or file perms issue) with "file doesn't exist". The "doesn't exist" path seems fine, there's no warnings, but a corrupt file also shows no issues unless you have the debug flag on.
Build the packages locally then create a corrupt package.json and try it:
❯ printf '%s\n' '{ "name": "demo", "dependencies": { "@outputai/output": "^0.9.0" } }' > package.json
❯ cat package.json| jq
jq: parse error: Expected another key-value pair at line 1, column 67
❯ node sdk/cli/bin/run.js update --cli
Latest Output SDK version: v0.9.0
Global install: v0.9.0 (up to date)
with debug on:
❯ DEBUG=output-cli:npm-update node sdk/cli/bin/run.js update --cli
Latest Output SDK version: v0.9.0
Global install: v0.9.0 (up to date)
output-cli:npm-update Failed to read local package.json: SyntaxError: Expected double-quoted property name in JSON at position 66 (line 1 column 67)
output-cli:npm-update at JSON.parse (<anonymous>)
output-cli:npm-update at readPackageJsonDependencies (file:///Users/clint/go/github.com/growthxai/output/.claude/worktrees/chore/remove_output_wrapper/sdk/cli/dist/services/npm_update_service.js:51:26)
| } | ||
|
|
||
| export function isPackageJsonVersionOutdated( version: string, latest: string ): boolean { | ||
| const range = semver.validRange( version, { includePrerelease: true } ); |
Collaborator
There was a problem hiding this comment.
does validRange() handle latest etc?
| return true; | ||
| } catch ( error: unknown ) { | ||
| this.warn( `Failed to update local install: ${getErrorMessage( error )}` ); | ||
| return false; |
Collaborator
There was a problem hiding this comment.
should we throw here? looking at the chain, it looks like we warn that we failed to install something but eventually exit with 0 status code in the CLI
| this.warn( `Some Output SDK packages are still behind v${latest}: ${staleNames}` ); | ||
| } | ||
| } else { | ||
| this.log( '\nLocal update completed (could not verify new versions)' ); |
Collaborator
There was a problem hiding this comment.
should we throw here? we log that we couldn't verify anything but then exit 0?
Comment on lines
+164
to
184
| export async function updateLocal( cwd: string, packageNames: string[], version: string ): Promise<void> { | ||
| const packages = packageNames.map( name => `${name}@${version}` ); | ||
| await spawnInherit( 'npm', [ 'install', '--ignore-scripts', '--save-exact', ...packages ], cwd ); | ||
| } | ||
|
|
||
| export async function updateLocalPackages( cwd: string, packages: LocalSdkPackage[], version: string ): Promise<void> { | ||
| const dependencies = packages | ||
| .filter( pkg => pkg.dependencyType === 'dependencies' ) | ||
| .map( pkg => `${pkg.name}@${version}` ); | ||
| const devDependencies = packages | ||
| .filter( pkg => pkg.dependencyType === 'devDependencies' ) | ||
| .map( pkg => `${pkg.name}@${version}` ); | ||
|
|
||
| if ( dependencies.length > 0 ) { | ||
| await spawnInherit( 'npm', [ 'install', '--ignore-scripts', '--save-exact', ...dependencies ], cwd ); | ||
| } | ||
|
|
||
| if ( devDependencies.length > 0 ) { | ||
| await spawnInherit( 'npm', [ 'install', '--ignore-scripts', '--save-dev', '--save-exact', ...devDependencies ], cwd ); | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
what is the difference between updateLocal() and updateLocalPackages() such that we need/want both?
|
|
||
| function findVersionInTree( deps: Record<string, any> | undefined ): string | null { | ||
| export interface LocalSdkPackage { | ||
| name: string; |
Collaborator
There was a problem hiding this comment.
nit: This can only ever be something from LOCAL_SDK_PACKAGE_NAMES, right? Should or could we tighten it from string?
name: typeof LOCAL_SDK_PACKAGE_NAMES[number]; // '@outputai/cli' | '@outputai/core' | ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@outputai/outputproject.output initto installs all packages directly.output updateto check each package version and update them individually.output init,output migrateandoutput dev.Reasoning
When installing the SDK modules via a third party dependency (
@outputai/output), they all become indirect dependencies (only that is actually present in thepackage.json).So, when the code imports "core" or "http", it is actually importing dependencies of a dependency, which is an anti-pattern in NPM and package managing in general.