Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monkeypatch remaining TS features within .gts files #791

Merged
merged 11 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 21 additions & 27 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
],
"args": [
"--extensionDevelopmentPath=${workspaceFolder}/packages/vscode",
"--disable-extensions", // uncomment to activate your local extensions
"${workspaceFolder}/test-packages"
]
},
Expand All @@ -36,26 +37,25 @@
"${workspaceFolder}/test-packages"
]
},
{
"name": "Debug Extension (Glint Only, No TS, Disable Ember LS)",
"type": "extensionHost",
"request": "launch",
"preLaunchTask": "npm: build",
"autoAttachChildProcesses": true,
"runtimeExecutable": "${execPath}",
"outFiles": [
"${workspaceFolder}/**/*.js",
"!**/node_modules/**"
],
"args": [
"--extensionDevelopmentPath=${workspaceFolder}/packages/vscode",
"--disable-extension",
"vscode.typescript-language-features",
"--disable-extension",
"lifeart.vscode-glimmer-syntax",
"${workspaceFolder}/test-packages"
]
},
// {
// "name": "Debug Extension (Glint Only, No TS, Disable Ember LS)",
// "type": "extensionHost",
// "request": "launch",
// "preLaunchTask": "npm: build",
// "autoAttachChildProcesses": true,
// "runtimeExecutable": "${execPath}",
// "outFiles": [
// "${workspaceFolder}/**/*.js",
// "!**/node_modules/**"
// ],
// "args": [
// "--extensionDevelopmentPath=${workspaceFolder}/packages/vscode",
// "--disable-extensions", // uncomment to activate your local extensions
// "--disable-extension",
// "vscode.typescript-language-features",
// "${workspaceFolder}/test-packages"
// ]
// },
{
// For this to work, make sure you're runninc `tsc --build --watch` at the root, AND
// `yarn bundle:watch` from within vscode directory.
Expand All @@ -71,20 +71,14 @@
],
"args": [
"--extensionDevelopmentPath=${workspaceFolder}/packages/vscode",
"--disable-extension",
"lifeart.vscode-glimmer-syntax",
"--disable-extension",
"unifiedjs.vscode-mdx",
"--disable-extension",
"Vue.volar",
"--disable-extensions", // uncomment to activate your local extensions
"${workspaceFolder}/test-packages/ts-plugin-test-app"
]
},
{
"name": "Attach to TS Server",
"type": "node",
"request": "attach",
"protocol": "inspector",
"port": 5667,
"sourceMaps": true,
"outFiles": [
Expand Down
17 changes: 17 additions & 0 deletions packages/typescript-plugin/src/typescript-server-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ const {
createAsyncLanguageServicePlugin,
} = require('@volar/typescript/lib/quickstart/createAsyncLanguageServicePlugin.js');

/**
* Volar provides two variants of functions for initializing a TS Plugin:
* - createLanguageServicePlugin
* - createAsyncLanguageServicePlugin
*
* The only difference is whether their setup callback is an async function or not.
* The reason we use the async variant is because of our use of `await import`, which
* we need in order to import the ESM glint package into our CJS VSCode extension.
*
* Unfortunately this singular tick of async appears to be causing a few race conditions,
* in particular that when freshly booting VSCode on a .gts file, there might not be
* any diagnostic messages until something "kicks" the TS Plugin to run, e.g.
* by editing the file.
*/
const plugin = createAsyncLanguageServicePlugin(
['.gts', '.gjs', '.hbs'],
(fileName: string) => {
Expand All @@ -18,7 +32,10 @@ const plugin = createAsyncLanguageServicePlugin(
return 3 satisfies ts.ScriptKind.TS;
},
async (_ts: any, info: any) => {
// The diagnostics race condition mentioned above appears to happen or at least
// be exacerbated by the fact that we use `await import` here.
const glintCore = await import('@glint/core');

const { findConfig, createEmberLanguagePlugin } = glintCore;

const cwd = info.languageServiceHost.getCurrentDirectory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Position,
CodeAction,
workspace,
TextEditor,
} from 'vscode';
import * as path from 'path';
import { describe, afterEach, test } from 'mocha';
Expand All @@ -25,15 +26,11 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
});

describe('diagnostics for errors', () => {
// TODO: fix remaining tests and remove this `.only`
test.only('with a custom extension', async () => {
test('with a custom extension', async () => {
let scriptURI = Uri.file(`${rootDir}/src/index.gts`);
let scriptEditor = await window.showTextDocument(scriptURI, { viewColumn: ViewColumn.One });

// Ensure we have a clean bill of health
expect(languages.getDiagnostics(scriptURI)).toEqual([]);

await hackishlyWaitForTypescriptPluginToActivate();
await hackishlyWaitForTypescriptPluginToActivate(scriptEditor, scriptURI);

// Replace a string with a number
await scriptEditor.edit((edit) => {
Expand Down Expand Up @@ -65,8 +62,7 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
// Open the script and the template
let scriptEditor = await window.showTextDocument(scriptURI, { viewColumn: ViewColumn.One });

// Ensure neither has any diagnostic messages
expect(languages.getDiagnostics(scriptURI)).toEqual([]);
await hackishlyWaitForTypescriptPluginToActivate(scriptEditor, scriptURI);

// Comment out a property in the script that's referenced in the template
await scriptEditor.edit((edit) => {
Expand All @@ -82,7 +78,7 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
new Range(new Position(10, 9), new Position(10, 9)),
);

expect(fixes.length).toBe(4);
expect(fixes.length).toBe(5);

const fix = fixes.find((fix) => fix.title === "Declare property 'undocumentedProperty'");

Expand All @@ -106,8 +102,7 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
// Open the script and the template
let scriptEditor = await window.showTextDocument(scriptURI, { viewColumn: ViewColumn.One });

// Ensure neither has any diagnostic messages
expect(languages.getDiagnostics(scriptURI)).toEqual([]);
await hackishlyWaitForTypescriptPluginToActivate(scriptEditor, scriptURI);

await scriptEditor.edit((edit) => {
edit.insert(new Position(10, 4), '{{this.localProp}} ');
Expand All @@ -122,7 +117,7 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
new Range(new Position(10, 12), new Position(10, 12)),
);

expect(fixes.length).toBe(4);
expect(fixes.length).toBe(5);

const fix = fixes.find((fix) => fix.title === "Declare property 'localProp'");

Expand All @@ -142,13 +137,56 @@ describe('Smoke test: ETI Environment (TS Plugin Mode)', () => {
});

/**
* We shouldn't have to use this function for many reasons:
* It takes a little while for the TS Plugin to fully activate, and unfortunately
* VSCode won't automatically re-trigger/re-calculate diagnostics for a file after
* a TS Plugin kicks in, so we need some way to know that the TS Plugin is activated
* before we edit the file.
*
* 1. Using timers to avoid a race condition is brittle
* 2. More importantly: this only solves the problem of "make sure the TS Plugin is activated
* before we edit the file" when what we REALLY want is diagnostics to kick in without
* editing.
* To accomplish this, this function inserts invalid TS into the .gts file and waits
* for diagnostics to show up.
*/
function hackishlyWaitForTypescriptPluginToActivate(): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, 5000));
async function hackishlyWaitForTypescriptPluginToActivate(
scriptEditor: TextEditor,
scriptURI: Uri,
): Promise<void> {
let invalidAssignment = 'let s: string = 123;';
await scriptEditor.edit((edit) => {
edit.insert(new Position(0, 0), invalidAssignment);
});

let numSpacesAdded = 0;
const startTime = Date.now();

// eslint-disable-next-line no-constant-condition
while (true) {
await scriptEditor.edit((edit) => {
edit.insert(new Position(0, 0), ' ');
});
numSpacesAdded++;

if (languages.getDiagnostics(scriptURI).length) {
break;
}

if (Date.now() - startTime > 5000) {
throw new Error(
'Timed out waiting for TS Plugin to activate (i.e. waiting for diagnostics to show up)',
);
}

// We'd love to wait for a smaller increment than 1000 but the editor
// debounces before triggering diagnostics so we need a large enough time.
await new Promise((resolve) => setTimeout(resolve, 1000));
}

// Remove our invalid assignment
await scriptEditor.edit((edit) => {
edit.replace(new Range(0, 0, 0, invalidAssignment.length + numSpacesAdded), '');
});

await new Promise((resolve) => setTimeout(resolve, 1000));

if (languages.getDiagnostics(scriptURI).length) {
throw new Error('Diagnostics still showing up after removing invalid assignment');
}
}
2 changes: 1 addition & 1 deletion packages/vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@
"@volar/vscode": "2.4.11",
"@vscode/test-electron": "^2.3.8",
"@vscode/vsce": "^2.22.0",
"esbuild": "^0.15.16",
"esbuild": "^0.24.0",
"expect": "^29.5.0",
"glob": "^10.2.4",
"mocha": "^10.2.0",
Expand Down
22 changes: 19 additions & 3 deletions packages/vscode/scripts/build.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
import { createRequire } from 'node:module';
import process from 'node:process';
import { fileURLToPath } from 'node:url';
import { build } from 'esbuild';
import { context } from 'esbuild';

const require = createRequire(import.meta.url);

const debug = process.argv.includes('--debug');
const watch = process.argv.includes('--watch');

await build({
// Create build context
const ctx = await context({
bundle: true,
entryPoints: {
'dist/extension': require.resolve('../lib/src/extension.js'),
Expand All @@ -21,5 +22,20 @@ await build({
platform: 'node',
sourcemap: debug,
target: 'node16',
watch: watch,
});

if (watch) {
// Start watch mode
await ctx.watch();
console.log('Watching for changes...');
} else {
// Do a single build
await ctx.rebuild();
await ctx.dispose();
}

// Handle SIGINT (Ctrl+C) gracefully
process.on('SIGINT', async () => {
await ctx.dispose();
process.exit(0);
});
82 changes: 82 additions & 0 deletions packages/vscode/src/extension.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import { createRequire } from 'node:module';
import * as path from 'node:path';
import * as lsp from '@volar/vscode/node';
import * as fs from 'node:fs';
import * as vscode from 'vscode';
import * as languageServerProtocol from '@volar/language-server/protocol.js';
import { createLabsInfo } from '@volar/vscode';
import { enabledHybridMode, enabledTypeScriptPlugin } from './hybrid-mode';
import { config } from './config';

import {
defineExtension,
Expand Down Expand Up @@ -205,3 +208,82 @@ function findLanguageServer(
return null;
}
}

try {
// The code below contains hacks lifted from the Vue extension to monkeypatch
// portions of official VSCode TS extension to add some missing features that make the tooling
// more seamless.
//
// https://github.com/vuejs/language-tools/blob/master/extensions/vscode/src/nodeClientMain.ts#L135-L195
//
// It is important for us (for the time being) to manually follow along with changes made to the
// Vue extension within the above file. Ideally Volar should extract this logic into a shared library.
//
// Specifically these hacks make things like Find File References, Go to Source Definition, etc.
// work in .gts files.
//
// https://github.com/search?q=repo%3Amicrosoft%2Fvscode%20isSupportedLanguageMode&type=code
const typeScriptServerPluginName = '@glint/typescript-plugin';

const tsExtension = vscode.extensions.getExtension('vscode.typescript-language-features')!;
const readFileSync = fs.readFileSync;
const extensionJsPath = require.resolve('./dist/extension.js', {
paths: [tsExtension.extensionPath],
});

const languageIdsQuoted = config.server.includeLanguages.map((lang) => `'${lang}'`).join(',');

// @ts-expect-error – not easy to type
fs.readFileSync = (...args) => {
if (args[0] === extensionJsPath) {
// @ts-expect-error – not easy to type
let text = readFileSync(...args) as string;

if (!enabledTypeScriptPlugin.value) {
// Make it possible to disable the TS server plugin if it's disabled by VSCode configuration.
// Default VSCode/TS doesn't make it possible to control this via configuration due to the
// fact that the TS plugin is declared in the extension's package.json's `typescriptServerPlugins`
text = text.replace(
'for(const e of n.contributes.typescriptServerPlugins',
(s) => s + `.filter(p=>p.name!=='${typeScriptServerPluginName}')`,
);
} else if (enabledHybridMode.value) {
// patch readPlugins
text = text.replace(
'languages:Array.isArray(e.languages)',
[
'languages:',
`e.name==='${typeScriptServerPluginName}'?[${languageIdsQuoted}]`,
':Array.isArray(e.languages)',
].join(''),
);

// VSCode < 1.87.0
text = text.replace(
't.$u=[t.$r,t.$s,t.$p,t.$q]',
(s) => s + `.concat(${languageIdsQuoted})`,
); // patch jsTsLanguageModes
text = text.replace(
'.languages.match([t.$p,t.$q,t.$r,t.$s]',
(s) => s + `.concat(${languageIdsQuoted})`,
); // patch isSupportedLanguageMode

// VSCode >= 1.87.0
text = text.replace(
't.jsTsLanguageModes=[t.javascript,t.javascriptreact,t.typescript,t.typescriptreact]',
(s) => s + `.concat(${languageIdsQuoted})`,
); // patch jsTsLanguageModes
text = text.replace(
'.languages.match([t.typescript,t.typescriptreact,t.javascript,t.javascriptreact]',
(s) => s + `.concat(${languageIdsQuoted})`,
); // patch isSupportedLanguageMode

return text;
}
}
// @ts-expect-error – not easy to type
return readFileSync(...args);
};
} catch {
// ignore
}
2 changes: 1 addition & 1 deletion test-packages/ts-plugin-test-app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"extends": "../tsconfig.compileroptions.json",
"extends": "../../tsconfig.compileroptions.json",
"compilerOptions": {
"baseUrl": ".",
"plugins": [{ "name": "@glint/typescript-plugin" }]
Expand Down
Loading
Loading