Skip to content

Conversation

@kevinramharak
Copy link

@kevinramharak kevinramharak commented Nov 4, 2025

Fixes #1213 (the ls part, not the )

changes:

  • update readme to be more accurate
  • add proper types instead of any
  • add the patchOptionsWithManifest call needed to make ls behave correctly

As it was very unclear what patchOptionsWithManifest did, so I scanned the source code to see where it is used and how. I found that only the publish and package (ls as a sub command of package) use the patchOptionsWithManifest function. Therefore I defined the types to reflect that. It is a bit ugly, because typescript does not like it when you are dynamically patching objects, especially with readonly properties, but I think its more accurate this way. I had to use 1 instance of any because typescript refuses to believe the properties were assignable, but nothing is changed in runtime semantics as any was used before the change.

I tried to add a test, but found there is no existing test (suite) for including options as part of the package.json, so was unsure where to add it. The related tests in package.test.ts focuss on testing the implementations and bypasses reading the options from the manifest file.

I tested it with:

# to build the local vsce binary
> npm run watch:build

# cwd: src/test/fixtures/devDependencies without `vsce` property in package.json
> node ../../../../vsce ls --tree
root-test-package-1.0.0.vsix
├─ package.json [0.2 KB]
└─ node_modules/
   ├─ real/
   │  ├─ dependency.js
   │  ├─ package.json [0.12 KB]
   │  └─ node_modules/
   │     └─ real_sub/
   │        ├─ dependency.js
   │        └─ package.json [0.05 KB]
   ├─ real2/
   │  ├─ dependency.js
   │  └─ package.json [0.2 KB]
   └─ real_sub/
      ├─ dependency.js
      └─ package.json [0.05 KB]


# cwd: src/test/fixtures/devDependencies with `{ vsce: { dependencies: true } }` in package.json
> node ../../../../vsce ls --tree
root-test-package-1.0.0.vsix
└─ package.json [0.24 KB]

The file package.json is large (0.24 KB)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The ls command does not respect the options in the manifest

1 participant