fix: strip proposed vscode-dts references from positron.d.ts (fixes #4)#6
Open
fix: strip proposed vscode-dts references from positron.d.ts (fixes #4)#6
Conversation
bb824de to
1fbadd1
Compare
Simulates a real npm consumer by creating a temp project that imports @posit-dev/positron and runs tsc against it. Tests three configurations: commonjs with skipLibCheck false (the issue #4 case), commonjs with skipLibCheck true, and Node16 module resolution.
Validates the built dist/positron.d.ts has no dangling vscode-dts reference directives and no proposed-only type references.
positron.d.ts contains `/// <reference path>` directives pointing at proposed VS Code API files that live in the Positron monorepo. These files are not shipped with the npm package, so consumers hit TS6053 errors and unresolved types (LanguageModelResponsePart2, etc.). During build, strip the reference directives and replace the 4 proposed-only types with `any`. Residual `X | any` unions are collapsed and `<T extends any = any>` is simplified to `<T = any>`. The build warns (but does not fail) if no directives are found, signaling the step can be removed once upstream changes. The affected surface area is limited to positron.ai.LanguageModelChatProvider, used exclusively by Positron LLM provider authors who have full types in the monorepo.
- Strip-proposed-types test now reads PROPOSED_TYPE_REPLACEMENTS from build.js instead of hardcoding the list, so they stay in sync - Integration test uses npm pack + npm install instead of symlinks, exercising the actual published tarball layout
- Use fs.cpSync instead of symlinks for @types/vscode fallback (Windows compatibility) - Add "type": "module" to the Node16 consumer project so it actually exercises ESM resolution
- Pack tarball to os.tmpdir() instead of hardcoded /tmp (portability) - Create @types parent directory before cpSync fallback (ENOENT fix)
Paths from os.tmpdir() may contain spaces; quote them in execSync shell strings to prevent argument splitting.
1fbadd1 to
6454635
Compare
nstrayer
commented
May 1, 2026
Comment on lines
+159
to
+218
| // ============================================================================= | ||
| // STEP 3.5: STRIP PROPOSED VSCODE-DTS REFERENCES FROM positron.d.ts | ||
| // ============================================================================= | ||
| // positron.d.ts contains `/// <reference path="../vscode-dts/...">` directives | ||
| // pointing at Positron-specific proposed VS Code API files. These files live in | ||
| // the Positron monorepo and are not shipped with this package, so consumers hit | ||
| // TS6053 ("file not found") errors (see issue #4). | ||
| // | ||
| // The types introduced by those proposed APIs are only used in a handful of | ||
| // provider-facing signatures (positron.ai.LanguageModelChatProvider). Rather | ||
| // than bundling the external files, we strip the reference directives and | ||
| // replace the proposed types with `any` so the file is self-contained. | ||
|
|
||
| console.log('\n📦 Step 3.5: Stripping proposed vscode-dts references from positron.d.ts...'); | ||
|
|
||
| const distPositronDts = path.join(PATHS.DIST_DIR, FILES.POSITRON_DTS); | ||
| const vscDtsRefRegex = /^\/\/\/\s*<reference\s+path=["'][^"']*vscode-dts\/[^"']*["']\s*\/>\s*\n?/gm; | ||
|
|
||
| const PROPOSED_TYPE_REPLACEMENTS = [ | ||
| 'LanguageModelChatInformation', | ||
| 'ProvideLanguageModelChatResponseOptions', | ||
| 'LanguageModelResponsePart2', | ||
| 'LanguageModelChatMessage2', | ||
| ]; | ||
|
|
||
| try { | ||
| let content = fs.readFileSync(distPositronDts, 'utf8'); | ||
|
|
||
| const refCount = (content.match(vscDtsRefRegex) || []).length; | ||
| if (refCount === 0) { | ||
| console.warn( | ||
| ' ⚠️ No vscode-dts reference directives found in dist/positron.d.ts; ' + | ||
| 'if upstream removed them, Step 3.5 can be deleted' | ||
| ); | ||
| } | ||
| content = content.replace(vscDtsRefRegex, ''); | ||
|
|
||
| for (const typeName of PROPOSED_TYPE_REPLACEMENTS) { | ||
| const typeRegex = new RegExp(`vscode\\.${typeName}`, 'g'); | ||
| content = content.replace(typeRegex, 'any'); | ||
| } | ||
|
|
||
| // Clean up residual `X | any` unions and `extends any` constraints left | ||
| // by the naive find-and-replace so the output reads intentionally. | ||
| content = content.replace(/\w[\w.]*(?:\s*\|\s*\w[\w.]*)*\s*\|\s*any/g, 'any'); | ||
| content = content.replace(/<T\s+extends\s+any\s*=\s*any>/g, '<T = any>'); | ||
|
|
||
| const leftover = content.match(/vscode-dts\//g); | ||
| if (leftover) { | ||
| throw new Error(`Stripped references but ${leftover.length} vscode-dts path(s) remain`); | ||
| } | ||
|
|
||
| fs.writeFileSync(distPositronDts, content); | ||
| console.log(` ✅ Removed ${refCount} vscode-dts reference directive(s)`); | ||
| console.log(` ✅ Replaced ${PROPOSED_TYPE_REPLACEMENTS.length} proposed type(s) with \`any\``); | ||
| } catch (error) { | ||
| console.error(` ❌ Failed to strip vscode-dts references: ${error.message}`); | ||
| process.exit(1); | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
This is the actual change/fix.
nstrayer
commented
May 1, 2026
| @@ -0,0 +1,113 @@ | |||
| import { describe, it, expect, beforeAll } from 'vitest'; | |||
Collaborator
Author
There was a problem hiding this comment.
A basic test set that should catch these things in the future.
nstrayer
commented
May 1, 2026
| @@ -0,0 +1,42 @@ | |||
| import { describe, it, expect } from 'vitest'; | |||
Collaborator
Author
There was a problem hiding this comment.
Was used as red-green TDD for this fix.
nstrayer
commented
May 1, 2026
| @@ -1,11 +1,8 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
Collaborator
Author
There was a problem hiding this comment.
This files' changes can be ignored because they're generated
nstrayer
commented
May 1, 2026
| @@ -1,8 +1,10 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
Collaborator
Author
There was a problem hiding this comment.
Also generated by the build script
juliasilge
approved these changes
May 2, 2026
juliasilge
left a comment
There was a problem hiding this comment.
This looks good and makes sense to me, although as you know, this is not my area of expertise.
Would it make sense to add some CI to run the tests? Not in this PR but sometime soon?
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
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
/// <reference path="...vscode-dts/...">directives fromdist/positron.d.tsand replaces 4 proposed-only types withany, making the published package self-containednpm packthe package, install it in a temp project, and runtsc --noEmitunder three configurations (commonjs + skipLibCheck false, commonjs + skipLibCheck true, Node16 ESM)build.js@types/vscodepeer dependency to^1.74.0(no version coupling)Closes #4.
Test plan
skipLibCheck: false