diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 994e90e..a50c016 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -6,18 +6,36 @@ labels: bug assignees: "" --- -**Describe the bug** +### **Describe the bug** + A clear and concise description of what the bug is. -**Expected behavior** +### **Expected behavior** + A clear and concise description of what you expected to happen. -**Original error** +### **Original error** + If this bug is related to an error that is not formatting well, please attach the original error in a code block: - + +``` Type 'number' is not assignable to type 'string'.ts(2322) - +``` + +### **Logs** + +Add the logs to help debugging what went wrong. See [these instructions](../../docs/hide-original-errors.md) on how to find and export the logs. + +Either add it as an external file or put them in between these `
` tags below:
+
+
+Logs +
+ + + + +### **Screenshots** -**Screenshots** If applicable, add screenshots to help explain your problem. diff --git a/apps/vscode-extension/src/commands/copyError.ts b/apps/vscode-extension/src/commands/copyError.ts index eaaa182..e44aa3a 100644 --- a/apps/vscode-extension/src/commands/copyError.ts +++ b/apps/vscode-extension/src/commands/copyError.ts @@ -1,13 +1,20 @@ import { commands, env, window, type ExtensionContext } from "vscode"; +import { execute } from "./execute"; + +const COMMAND_ID = "prettyTsErrors.copyError"; export function registerCopyError(context: ExtensionContext) { context.subscriptions.push( - commands.registerCommand( - "prettyTsErrors.copyError", - async (errorMessage: string) => { + commands.registerCommand(COMMAND_ID, async (errorMessage: unknown) => + execute(COMMAND_ID, async () => { + if (typeof errorMessage !== "string") { + throw new Error("cannot write non-string value to clipboard", { + cause: errorMessage, + }); + } await env.clipboard.writeText(errorMessage); window.showInformationMessage("Copied error message to clipboard!"); - } + }) ) ); } diff --git a/apps/vscode-extension/src/commands/execute.ts b/apps/vscode-extension/src/commands/execute.ts new file mode 100644 index 0000000..01ded82 --- /dev/null +++ b/apps/vscode-extension/src/commands/execute.ts @@ -0,0 +1,24 @@ +import { window } from "vscode"; +import { logger } from "../logger"; + +/** + * A wrapper function to execute command tasks while providing user feedback and logging on errors. + */ +export async function execute( + commandName: string, + task: (...args: unknown[]) => unknown | Promise +) { + try { + return await task(); + } catch (error) { + if (error instanceof Error) { + logger.error(error); + } else if (typeof error === "string") { + logger.error(error); + } else { + logger.error("caught non-string or error value: ", error); + } + window.showErrorMessage(`Failed to execute command: '${commandName}'`); + throw error; + } +} diff --git a/apps/vscode-extension/src/commands/openMarkdownPreview.ts b/apps/vscode-extension/src/commands/openMarkdownPreview.ts new file mode 100644 index 0000000..c567deb --- /dev/null +++ b/apps/vscode-extension/src/commands/openMarkdownPreview.ts @@ -0,0 +1,24 @@ +import type { ExtensionContext } from "vscode"; +import { commands } from "vscode"; +import { MarkdownWebviewProvider } from "../provider/markdownWebviewProvider"; +import { tryEnsureUri } from "./validate"; +import { execute } from "./execute"; + +const COMMAND_ID = "prettyTsErrors.openMarkdownPreview"; + +export function registerOpenMarkdownPreview(context: ExtensionContext) { + const provider = new MarkdownWebviewProvider(context); + context.subscriptions.push( + commands.registerCommand(COMMAND_ID, async (maybeUriLike: unknown) => + execute(COMMAND_ID, async () => { + const { isValidUri, uri } = tryEnsureUri(maybeUriLike); + if (!isValidUri) { + throw new Error("cannot open markdown preview with an invalid uri", { + cause: maybeUriLike, + }); + } + await provider.openMarkdownPreview(uri); + }) + ) + ); +} diff --git a/apps/vscode-extension/src/commands/revealSelection.ts b/apps/vscode-extension/src/commands/revealSelection.ts index 7773c07..e6ebb8b 100644 --- a/apps/vscode-extension/src/commands/revealSelection.ts +++ b/apps/vscode-extension/src/commands/revealSelection.ts @@ -1,7 +1,7 @@ import { type ExtensionContext, + type Tab, commands, - Range, TabInputText, TabInputWebview, Uri, @@ -9,64 +9,86 @@ import { window, } from "vscode"; import { MarkdownWebviewProvider } from "../provider/markdownWebviewProvider"; +import { tryEnsureRange, tryEnsureUri } from "./validate"; +import { execute } from "./execute"; + +const COMMAND_ID = "prettyTsErrors.revealSelection"; export function registerRevealSelection(context: ExtensionContext) { context.subscriptions.push( commands.registerCommand( - "prettyTsErrors.revealSelection", - async (uri: Uri, range: Range) => { - // ensure these are real instances - uri = Uri.from({ ...uri }); - range = new Range( - range.start.line, - range.start.character, - range.end.line, - range.end.character - ); - - // default behaviour is to use the active view column - let viewColumn = ViewColumn.Active; - - // detect if the active tab is our preview webview - let isFromMarkdownPreviewWebview = false; - const activeTab = window.tabGroups.activeTabGroup.activeTab; - if (activeTab && activeTab.input instanceof TabInputWebview) { - // For an unknown reason this string is prefixed with something like `mainthread-${viewType}` - // endsWith should handle a full match and the prefixed versions - if ( - activeTab.input.viewType.endsWith(MarkdownWebviewProvider.viewType) - ) { - isFromMarkdownPreviewWebview = true; + COMMAND_ID, + async (maybeUriLike: unknown, maybeRangeLike: unknown) => + execute(COMMAND_ID, async () => { + const { isValidUri, uri } = tryEnsureUri(maybeUriLike); + const { isValidRange, range } = tryEnsureRange(maybeRangeLike); + if (!isValidUri || !isValidRange) { + throw new Error( + "cannot reveal selection with invalid range or uri", + { + cause: { + range: maybeRangeLike, + uri: maybeUriLike, + }, + } + ); } - } - - if (isFromMarkdownPreviewWebview) { - // find a tab group where the file is open, then use that view column for the `vscode.open` command - const tabs = window.tabGroups.all.flatMap( - (tabGroup) => tabGroup.tabs - ); - const tabWithFileOpen = tabs.find((tab) => { - if (tab.input instanceof TabInputText) { - return tab.input.uri.toString() === uri.toString(); - } - return false; + const viewColumn = determineViewColumn(uri); + return commands.executeCommand("vscode.open", uri, { + selection: range, + viewColumn, }); - if (tabWithFileOpen) { - viewColumn = tabWithFileOpen.group.viewColumn; - } else { - // If markdown preview is not open on 1, open the link in 1, else open the link in 2 - viewColumn = - activeTab!.group.viewColumn !== 1 - ? ViewColumn.One - : ViewColumn.Two; - } - } - - return commands.executeCommand("vscode.open", uri, { - selection: range, - viewColumn, - }); - } + }) ) ); } + +/** + * Determine the view column to use for the `vscode.open` command: + * - if the command is not called from a markdown webview preview, default to `ViewColumn.Active`, which will defer the decision to VS Code + * - else check if the file is open somewhere and use its view column + * - else use the opposite of where the preview resides (left if the preview is right, right if the preview is left) + * + * This seems a bit complex, but without it VS Code will open a new view column to the right and open the given uri in it, regardless of the current layout and open files. + * Using this algorithm makes it feel more intuitive and less stupid. + */ +function determineViewColumn(uri: Uri): ViewColumn { + if (!isFromMarkdownPreviewWebview()) { + return ViewColumn.Active; + } + const tab = findTabWithFileOpen(uri); + if (tab) { + return tab.group.viewColumn; + } + // If markdown preview is not open on 1, open the link in 1, else open the link in 2 + const activeTab = window.tabGroups.activeTabGroup.activeTab; + return activeTab!.group.viewColumn !== 1 ? ViewColumn.One : ViewColumn.Two; +} + +/** + * Search for an open tab with the given `uri` and return it if it exists + */ +function findTabWithFileOpen(uri: Uri): Tab | undefined { + const tabs = window.tabGroups.all.flatMap((tabGroup) => tabGroup.tabs); + return tabs.find((tab) => { + if (tab.input instanceof TabInputText) { + return tab.input.uri.toString() === uri.toString(); + } + return false; + }); +} + +/** + * Returns `true` if the active tab is a pretty-ts-errors markdown preview webview + */ +function isFromMarkdownPreviewWebview(): boolean { + const activeTab = window.tabGroups.activeTabGroup.activeTab; + if (activeTab && activeTab.input instanceof TabInputWebview) { + // For an unknown reason this string is prefixed with something like `mainthread-${viewType}` + // endsWith should handle a full match and the prefixed versions + if (activeTab.input.viewType.endsWith(MarkdownWebviewProvider.viewType)) { + return true; + } + } + return false; +} diff --git a/apps/vscode-extension/src/commands/validate.ts b/apps/vscode-extension/src/commands/validate.ts new file mode 100644 index 0000000..df8c468 --- /dev/null +++ b/apps/vscode-extension/src/commands/validate.ts @@ -0,0 +1,79 @@ +import { Range, Uri } from "vscode"; + +export function tryEnsureUri( + maybeUriLike: unknown +): { isValidUri: true; uri: Uri } | { isValidUri: false; uri?: undefined } { + if (maybeUriLike instanceof Uri) { + return { isValidUri: true, uri: maybeUriLike }; + } + if (typeof maybeUriLike === "string") { + try { + return { isValidUri: true, uri: Uri.parse(maybeUriLike, true) }; + } catch (error) { + return { isValidUri: false }; + } + } + if (isUriLike(maybeUriLike)) { + return { isValidUri: true, uri: Uri.from(maybeUriLike) }; + } + return { isValidUri: false }; +} + +type UriLike = Parameters[0]; + +function isUriLike(value: unknown): value is UriLike { + return ( + typeof value === "object" && + value != null && + "scheme" in value && + typeof value.scheme === "string" + ); +} + +export function tryEnsureRange( + maybeRangeLike: unknown +): + | { isValidRange: true; range: Range } + | { isValidRange: false; range?: undefined } { + if (maybeRangeLike instanceof Range) { + return { isValidRange: true, range: maybeRangeLike }; + } + if (isRangeLike(maybeRangeLike)) { + return { + isValidRange: true, + range: new Range( + maybeRangeLike.start.line, + maybeRangeLike.start.character, + maybeRangeLike.end.line, + maybeRangeLike.end.character + ), + }; + } + return { isValidRange: false }; +} + +type RangeLike = { start: PositionLike; end: PositionLike }; + +function isRangeLike(value: unknown): value is RangeLike { + return ( + typeof value === "object" && + value != null && + "start" in value && + isPositionLike(value.start) && + "end" in value && + isPositionLike(value.end) + ); +} + +type PositionLike = { line: number; character: number }; + +function isPositionLike(value: unknown): value is PositionLike { + return ( + typeof value === "object" && + value !== null && + "line" in value && + typeof value.line === "number" && + "character" in value && + typeof value.character === "number" + ); +} diff --git a/apps/vscode-extension/src/diagnostics.ts b/apps/vscode-extension/src/diagnostics.ts new file mode 100644 index 0000000..fc270e7 --- /dev/null +++ b/apps/vscode-extension/src/diagnostics.ts @@ -0,0 +1,146 @@ +import { has } from "@pretty-ts-errors/utils"; +import { formatDiagnostic } from "@pretty-ts-errors/vscode-formatter"; +import { + ExtensionContext, + languages, + MarkdownString, + window, + Uri, + type Diagnostic, +} from "vscode"; +import { + createConverter, + type Converter, +} from "vscode-languageclient/lib/common/codeConverter"; +import { hoverProvider } from "./provider/hoverProvider"; +import { + formattedDiagnosticsStore, + type FormattedDiagnostic, +} from "./formattedDiagnosticsStore"; +import { logger } from "./logger"; + +/** + * The list of diagnostic sources that pretty-ts-errors supports + */ +const supportedDiagnosticSources = [ + "ts", + "ts-plugin", + "deno-ts", + "js", + "glint", +]; + +export function registerOnDidChangeDiagnostics(context: ExtensionContext) { + const converter = createConverter(); + context.subscriptions.push( + languages.onDidChangeDiagnostics(async (e) => { + logger.measure("onDidChangeDiagnostics", () => { + e.uris.forEach((uri) => { + logger.measure(`diagnostics for: ${uri.toString(true)}`, () => { + const diagnostics = languages.getDiagnostics(uri); + const supportedDiagnostics = diagnostics.filter( + (diagnostic) => + diagnostic.source && + has(supportedDiagnosticSources, diagnostic.source) + ); + + const items: FormattedDiagnostic[] = supportedDiagnostics.map( + (diagnostic) => getFormattedDiagnostic(diagnostic, uri, converter) + ); + + // TODO: we should check if never deleting the entries is a performance issue + // probably not, since solving all diagnostics for a file should set its value to an empty collection, but we should check anyway + // see: https://github.com/yoavbls/pretty-ts-errors/issues/139 + formattedDiagnosticsStore.set(uri.fsPath, items); + + if (items.length > 0) { + ensureHoverProviderIsRegistered(uri, context); + } + }); + }); + }); + }) + ); +} + +/** + * To prevent infinite memory consumtion use a max size for the cache + * + * TODO: consider making this configurable to the end user with a sensible `min` and `max` + */ +const CACHE_SIZE_MAX = 100; + +/** + * A local cache that maps TS diagnostics as `string` to their formatted `MarkdownString` counter part. + * @see https://github.com/yoavbls/pretty-ts-errors/pull/62 + * + * One reason this cache is critical is because the TypeScript Language Features extension is very noisy and will constantly push all diagnostics for a file, + * even if there were no actual changes. + * @see https://github.com/yoavbls/pretty-ts-errors/issues/139#issuecomment-3401279357 + * + * TODO: create a proper LRU cache, to prevent exceeding the cache size being a bottleneck + * @see https://github.com/yoavbls/pretty-ts-errors/issues/104 + */ +const cache = new Map(); + +function getFormattedDiagnostic( + diagnostic: Diagnostic, + uri: Uri, + converter: Converter +): FormattedDiagnostic { + let formattedMessage = cache.get(diagnostic.message); + + if (!formattedMessage) { + // formatDiagnostic converts message based on LSP Diagnostic type, not VSCode Diagnostic type, so it can be used in other IDEs. + // Here we convert VSCode Diagnostic to LSP Diagnostic to make formatDiagnostic recognize it. + const lspDiagnostic = converter.asDiagnostic(diagnostic); + const formattedDiagnostic = logger.measure( + `formatDiagnostic(\`${lspDiagnostic.message}\`)`, + () => formatDiagnostic(lspDiagnostic, { uri }) + ); + const markdownString = new MarkdownString(formattedDiagnostic); + + // TODO: consider using the `{ enabledCommands: string[] }` variant, to only allow whitelisted commands + markdownString.isTrusted = true; + markdownString.supportHtml = true; + + formattedMessage = markdownString; + if (cache.size > CACHE_SIZE_MAX) { + const firstCacheKey = cache.keys().next().value!; + cache.delete(firstCacheKey); + } + cache.set(diagnostic.message, formattedMessage); + } + + return { + range: diagnostic.range, + contents: [formattedMessage], + }; +} + +/** + * A set to prevent registering duplicate hover providers. + */ +const registeredLanguages = new Set(); + +/** + * Ensure a hover provider is registered for any visible editors where pretty-ts-errors has a formatted diagnostic + */ +function ensureHoverProviderIsRegistered(uri: Uri, context: ExtensionContext) { + const editor = window.visibleTextEditors.find( + (editor) => editor.document.uri.toString() === uri.toString() + ); + const languageId = editor?.document.languageId; + if (languageId && !registeredLanguages.has(languageId)) { + logger.debug(`registering hover provider for language id: ${languageId}`); + registeredLanguages.add(languageId); + context.subscriptions.push( + languages.registerHoverProvider( + { + language: languageId, + }, + hoverProvider + ) + ); + } +} diff --git a/apps/vscode-extension/src/extension.ts b/apps/vscode-extension/src/extension.ts index 31453b0..7345532 100644 --- a/apps/vscode-extension/src/extension.ts +++ b/apps/vscode-extension/src/extension.ts @@ -1,105 +1,33 @@ -import { has } from "@pretty-ts-errors/utils"; -import { formatDiagnostic } from "@pretty-ts-errors/vscode-formatter"; -import { - ExtensionContext, - languages, - MarkdownString, - Range, - window, -} from "vscode"; -import { createConverter } from "vscode-languageclient/lib/common/codeConverter"; -import { hoverProvider } from "./provider/hoverProvider"; +import { ExtensionContext } from "vscode"; +import { registerOnDidChangeDiagnostics } from "./diagnostics"; +import { logger } from "./logger"; +import { registerCopyError } from "./commands/copyError"; +import { registerRevealSelection } from "./commands/revealSelection"; +import { registerOpenMarkdownPreview } from "./commands/openMarkdownPreview"; import { registerSelectedTextHoverProvider } from "./provider/selectedTextHoverProvider"; -import { uriStore } from "./provider/uriStore"; import { registerTextDocumentProvider } from "./provider/textDocumentContentProvider"; -import { registerMarkdownWebviewProvider } from "./provider/markdownWebviewProvider"; -import { registerRevealSelection } from "./commands/revealSelection"; import { registerWebviewViewProvider } from "./provider/webviewViewProvider"; -import { registerCopyError } from "./commands/copyError"; - -const cache = new Map(); export function activate(context: ExtensionContext) { - const registeredLanguages = new Set(); - const converter = createConverter(); + logger.info("activating"); + // logging and debug features + logger.register(context); registerSelectedTextHoverProvider(context); - registerTextDocumentProvider(context); - registerMarkdownWebviewProvider(context); - registerWebviewViewProvider(context); - registerRevealSelection(context); - registerCopyError(context); - - context.subscriptions.push( - languages.onDidChangeDiagnostics(async (e) => { - e.uris.forEach((uri) => { - const diagnostics = languages.getDiagnostics(uri); - - const items: { - range: Range; - contents: MarkdownString[]; - }[] = []; - let hasTsDiagnostic = false; + // prettify diagnostics feature + registerOnDidChangeDiagnostics(context); - diagnostics - .filter((diagnostic) => - diagnostic.source - ? has( - ["ts", "ts-plugin", "deno-ts", "js", "glint"], - diagnostic.source - ) - : false - ) - .forEach(async (diagnostic) => { - // formatDiagnostic converts message based on LSP Diagnostic type, not VSCode Diagnostic type, so it can be used in other IDEs. - // Here we convert VSCode Diagnostic to LSP Diagnostic to make formatDiagnostic recognize it. - let formattedMessage = cache.get(diagnostic.message); - - if (!formattedMessage) { - const markdownString = new MarkdownString( - formatDiagnostic(converter.asDiagnostic(diagnostic), { uri }) - ); - - markdownString.isTrusted = true; - markdownString.supportHtml = true; - - formattedMessage = markdownString; - cache.set(diagnostic.message, formattedMessage); - - if (cache.size > 100) { - const firstCacheKey = cache.keys().next().value; - cache.delete(firstCacheKey); - } - } - - items.push({ - range: diagnostic.range, - contents: [formattedMessage], - }); - - hasTsDiagnostic = true; - }); + // UI elements that show the prettified diagnostics + registerTextDocumentProvider(context); + registerWebviewViewProvider(context); - uriStore[uri.fsPath] = items; + // register commands + registerCopyError(context); + registerOpenMarkdownPreview(context); + registerRevealSelection(context); +} - if (hasTsDiagnostic) { - const editor = window.visibleTextEditors.find( - (editor) => editor.document.uri.toString() === uri.toString() - ); - if (editor && !registeredLanguages.has(editor.document.languageId)) { - registeredLanguages.add(editor.document.languageId); - context.subscriptions.push( - languages.registerHoverProvider( - { - language: editor.document.languageId, - }, - hoverProvider - ) - ); - } - } - }); - }) - ); +export function deactivate() { + logger.info("deactivating"); } diff --git a/apps/vscode-extension/src/formattedDiagnosticsStore.ts b/apps/vscode-extension/src/formattedDiagnosticsStore.ts new file mode 100644 index 0000000..9e6b078 --- /dev/null +++ b/apps/vscode-extension/src/formattedDiagnosticsStore.ts @@ -0,0 +1,18 @@ +import { MarkdownString, Range, Uri } from "vscode"; + +type StoreKey = Uri["fsPath"]; + +export interface FormattedDiagnostic { + range: Range; + contents: MarkdownString[]; +} + +/** + * A store for formatted diagnostics, where the key is the file path to a file, and the value a collection of formatted diagnostics for that file. + * + * The `onDidChangeDiagnostics` event handler will fill the store with formatted diagnostics, while other components will query the store to display these diagnostics. + */ +export const formattedDiagnosticsStore = new Map< + StoreKey, + FormattedDiagnostic[] +>(); diff --git a/apps/vscode-extension/src/logger.ts b/apps/vscode-extension/src/logger.ts new file mode 100644 index 0000000..dd2bb65 --- /dev/null +++ b/apps/vscode-extension/src/logger.ts @@ -0,0 +1,143 @@ +import { + ExtensionMode, + LogOutputChannel, + window, + type ExtensionContext, + LogLevel as VSLogLevel, +} from "vscode"; + +let instance: null | LogOutputChannel = null; + +function getLogger(): LogOutputChannel { + if (instance !== null) { + return instance; + } + instance = window.createOutputChannel("Pretty TypeScript Errors", { + log: true, + }); + return instance; +} + +function info(...args: Parameters) { + getLogger().info(...args); +} + +function trace(...args: Parameters) { + getLogger().trace(...args); +} + +function debug(...args: Parameters) { + getLogger().debug(...args); +} +function warn(...args: Parameters) { + getLogger().warn(...args); +} + +function error(...args: Parameters) { + getLogger().error(...args); +} + +type LogLevel = "info" | "trace" | "debug" | "warn" | "error"; +type LogLevelThresholds = Record; +type SortedLogLevelThresholds = [LogLevel, number][]; + +const defaultThresholds: LogLevelThresholds = { + error: 5000, + warn: 1000, + info: 100, + debug: 50, + trace: 0, +}; + +/** + * Both in the browser and Node >= 16 (vscode 1.77 has node >= 16) have `performance` available as a global + * But `@types/node` is missing its global declaration, this fixes the type error we get from using it + */ +declare const performance: import("perf_hooks").Performance; + +/** + * Measures the time it took to run `task` and reports it to the `logger` based on `logLevelThresholds`. + * + * NOTE: supports synchronous `task`s only + * @see {@link defaultThresholds} for the default thresholds + */ +function measure( + name: string, + task: () => T, + logLevelThresholds: Partial = {} +): T { + const start = performance.now(); + const result = task(); + if (isPromiseLike(result)) { + logger.warn("cannot log.measure async tasks", task, result); + return result; + } + const end = performance.now(); + const duration = end - start; + const thresholds = normalizeThresholds(logLevelThresholds); + const level = findLogLevel(thresholds, duration); + getLogger()[level](`task ${name} took ${duration.toFixed(3)}ms`); + return result; +} + +function normalizeThresholds( + logLevelThresholds: Partial +): SortedLogLevelThresholds { + logLevelThresholds = Object.assign({}, defaultThresholds, logLevelThresholds); + // sort thresholds from high to low + // { info: 100, warn: 1000, trace: 0 } => [[warn, 1000], [info, 100], [trace, 0]] + return Object.entries(logLevelThresholds).sort( + ([_a, a], [_b, b]) => b - a + ) as SortedLogLevelThresholds; +} + +function findLogLevel( + thresholds: SortedLogLevelThresholds, + duration: number, + defaultLogLevel: LogLevel = "debug" +): LogLevel { + return ( + thresholds.find(([_, threshold]) => duration > threshold)?.[0] ?? + defaultLogLevel + ); +} + +function dispose() { + if (instance !== null) { + instance.dispose(); + instance = null; + } +} + +function isPromiseLike(value: unknown): value is PromiseLike { + return ( + value != null && + typeof value === "object" && + "then" in value && + typeof value["then"] === "function" + ); +} + +function register(context: ExtensionContext) { + if (context.extensionMode === ExtensionMode.Development) { + const instance = getLogger(); + instance.show(); + if (instance.logLevel !== VSLogLevel.Trace) { + instance.appendLine( + `To see more verbose logging, set this output's log level to "Trace" (gear icon next to the dropdown).` + ); + } + } + context.subscriptions.push({ dispose }); +} + +export const logger = { + trace, + debug, + info, + warn, + error, + measure, + register, + dispose, +}; diff --git a/apps/vscode-extension/src/provider/hoverProvider.ts b/apps/vscode-extension/src/provider/hoverProvider.ts index 9662fff..ffb19d6 100644 --- a/apps/vscode-extension/src/provider/hoverProvider.ts +++ b/apps/vscode-extension/src/provider/hoverProvider.ts @@ -1,17 +1,15 @@ import { HoverProvider } from "vscode"; -import { uriStore } from "./uriStore"; +import { formattedDiagnosticsStore } from "../formattedDiagnosticsStore"; export const hoverProvider: HoverProvider = { provideHover(document, position, _token) { - const itemsInUriStore = uriStore[document.uri.fsPath]; + const items = formattedDiagnosticsStore.get(document.uri.fsPath); - if (!itemsInUriStore) { + if (!items) { return null; } - const itemInRange = itemsInUriStore.filter((item) => - item.range.contains(position) - ); + const itemInRange = items.filter((item) => item.range.contains(position)); if (itemInRange.length === 0) { return null; diff --git a/apps/vscode-extension/src/provider/markdownWebviewProvider.ts b/apps/vscode-extension/src/provider/markdownWebviewProvider.ts index 1c2476a..2e795b2 100644 --- a/apps/vscode-extension/src/provider/markdownWebviewProvider.ts +++ b/apps/vscode-extension/src/provider/markdownWebviewProvider.ts @@ -1,33 +1,16 @@ -import type { ExtensionContext } from "vscode"; import * as vscode from "vscode"; import { getUserLangs, getUserTheme } from "vscode-shiki-bridge"; import { HighlighterCore, createHighlighterCore } from "shiki/core"; import { createOnigurumaEngine } from "shiki/engine/oniguruma"; import { createMarkdownExit, type MarkdownExit } from "markdown-exit"; - -export function registerMarkdownWebviewProvider(context: ExtensionContext) { - const provider = new MarkdownWebviewProvider(context); - context.subscriptions.push( - vscode.commands.registerCommand( - "prettyTsErrors.openMarkdownPreview", - async (uri: vscode.Uri) => { - if (!uri || !(uri instanceof vscode.Uri)) { - console.error( - "expected uri to be an instance of vscode.Uri, instead got: ", - uri - ); - return; - } - await provider.openMarkdownPreview(uri); - } - ) - ); -} +import { PRETTY_TS_ERRORS_SCHEME } from "./textDocumentContentProvider"; function createMarkdownExitPatched( highlight: (code: string) => Promise ) { const md = createMarkdownExit({ + // TODO: try and refactor this to not use a markdown renderer, this would remove this possible XSS attack vector + // @see https://github.com/yoavbls/pretty-ts-errors/pull/152#issuecomment-3690510196 html: true, highlight, }); @@ -120,6 +103,12 @@ export class MarkdownWebviewProvider { } async openMarkdownPreview(uri: vscode.Uri) { + // until the markdown renderer has been removed, try and avoid rendering unknown uri's to limit the chance of someone finding an XSS vulnerability. + if (uri.scheme !== PRETTY_TS_ERRORS_SCHEME) { + throw new Error( + `cannot provide a markdown preview for a non '${PRETTY_TS_ERRORS_SCHEME}' scheme uri` + ); + } const document = await vscode.workspace.openTextDocument(uri); const markdown = document.getText(); const content = await this.renderMarkdown(markdown); diff --git a/apps/vscode-extension/src/provider/selectedTextHoverProvider.ts b/apps/vscode-extension/src/provider/selectedTextHoverProvider.ts index 090b1d4..e6fb059 100644 --- a/apps/vscode-extension/src/provider/selectedTextHoverProvider.ts +++ b/apps/vscode-extension/src/provider/selectedTextHoverProvider.ts @@ -8,19 +8,18 @@ import { window, } from "vscode"; import { createConverter } from "vscode-languageclient/lib/common/codeConverter"; -import { uriStore } from "./uriStore"; +import { formattedDiagnosticsStore } from "../formattedDiagnosticsStore"; /** * Register an hover provider in debug only. * It format selected text and help test things visually easier. */ export function registerSelectedTextHoverProvider(context: ExtensionContext) { - const converter = createConverter(); - if (context.extensionMode !== ExtensionMode.Development) { return; } + const converter = createConverter(); context.subscriptions.push( languages.registerHoverProvider( { @@ -30,6 +29,11 @@ export function registerSelectedTextHoverProvider(context: ExtensionContext) { { provideHover(document, position) { const editor = window.activeTextEditor; + + if (!editor) { + return; + } + const range = document.getWordRangeAtPosition(position); const message = editor ? document.getText(editor.selection) : ""; @@ -54,7 +58,9 @@ export function registerSelectedTextHoverProvider(context: ExtensionContext) { markdown.isTrusted = true; markdown.supportHtml = true; - uriStore[document.uri.fsPath] = [{ range, contents: [markdown] }]; + formattedDiagnosticsStore.set(document.uri.fsPath, [ + { range, contents: [markdown] }, + ]); return { contents: [markdown], diff --git a/apps/vscode-extension/src/provider/textDocumentContentProvider.ts b/apps/vscode-extension/src/provider/textDocumentContentProvider.ts index bbfc6ca..74baac1 100644 --- a/apps/vscode-extension/src/provider/textDocumentContentProvider.ts +++ b/apps/vscode-extension/src/provider/textDocumentContentProvider.ts @@ -4,7 +4,8 @@ import { workspace, type TextDocumentContentProvider, } from "vscode"; -import { uriStore } from "./uriStore"; +import { formattedDiagnosticsStore } from "../formattedDiagnosticsStore"; +import { logger } from "../logger"; export const PRETTY_TS_ERRORS_SCHEME = "pretty-ts-errors"; @@ -29,14 +30,19 @@ export const textDocumentContentProvider: TextDocumentContentProvider = { provideTextDocumentContent(uri): string { const searchParams = new URLSearchParams(uri.query); if (!uri.fsPath.endsWith(".md")) { - return `only supports .md file extensions for ${uri.fsPath}`; + logger.error( + `Tried to provide a text document for fsPath: '${uri.fsPath}'` + ); + return `only supports .md file extensions (given ${uri.fsPath})`; } const fsPath = uri.fsPath.slice(0, -3); - const items = uriStore[fsPath]; + const items = formattedDiagnosticsStore.get(fsPath); if (!items) { + logger.error(`No diagnostics found for '${fsPath}'`); return `no diagnostics found for ${fsPath}`; } if (!searchParams.has("range")) { + logger.error(`Missing range query parameter for '${uri}'`); return `range query parameter is missing for uri: ${uri}`; } const range = createRangeFromString(searchParams.get("range")!); @@ -44,9 +50,12 @@ export const textDocumentContentProvider: TextDocumentContentProvider = { return item.range.isEqual(range); }); if (!item) { - return `no diagnostic found for ${fsPath} with range ${JSON.stringify( - range - )}`; + const rangeAsString = JSON.stringify(range); + logger.error( + `No diagnostic found for '${fsPath}' and range '${rangeAsString}', items:`, + items + ); + return `no diagnostic found for ${fsPath} with range ${rangeAsString}`; } return item.contents.map((content) => content.value).join("\n"); }, diff --git a/apps/vscode-extension/src/provider/uriStore.ts b/apps/vscode-extension/src/provider/uriStore.ts deleted file mode 100644 index ec87bcc..0000000 --- a/apps/vscode-extension/src/provider/uriStore.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { MarkdownString, Range, Uri } from "vscode"; - -export const uriStore: Record< - Uri["path"], - { - range: Range; - contents: MarkdownString[]; - }[] -> = {}; diff --git a/apps/vscode-extension/src/provider/webviewViewProvider.ts b/apps/vscode-extension/src/provider/webviewViewProvider.ts index d5157d6..24db77e 100644 --- a/apps/vscode-extension/src/provider/webviewViewProvider.ts +++ b/apps/vscode-extension/src/provider/webviewViewProvider.ts @@ -1,8 +1,11 @@ import type { ExtensionContext } from "vscode"; import * as vscode from "vscode"; import { MarkdownWebviewProvider } from "./markdownWebviewProvider"; -import { uriStore } from "./uriStore"; -import { has } from "packages/utils"; +import { + formattedDiagnosticsStore, + type FormattedDiagnostic, +} from "../formattedDiagnosticsStore"; +import { has } from "@pretty-ts-errors/utils"; const SUPPORTED_LANGUAGE_IDS = [ "typescript", @@ -23,11 +26,10 @@ export function registerWebviewViewProvider(context: ExtensionContext) { ); } -type CachedDiagnostic = (typeof uriStore)[string][number]; - +// TODO: adding a `MarkdownWebviewView` class would make this provider a lot simpler class MarkdownWebviewViewProvider implements vscode.WebviewViewProvider { private disposables = new Map(); - private shownDiagnostics = new WeakMap(); + private shownDiagnostics = new WeakMap(); constructor(private readonly provider: MarkdownWebviewProvider) {} async resolveWebviewView( @@ -48,6 +50,7 @@ class MarkdownWebviewViewProvider implements vscode.WebviewViewProvider { this.provider.createOnDidReceiveMessage() ), vscode.languages.onDidChangeDiagnostics(() => + // TODO: since `onDidChangeDiagnostics` fires often, we should try and avoid calling refresh based on the event uris this.refresh(webviewView.webview) ), vscode.window.onDidChangeActiveTextEditor((editor) => { @@ -90,7 +93,7 @@ class MarkdownWebviewViewProvider implements vscode.WebviewViewProvider { const selection = activeEditor?.selection; if (activeEditor && selection) { const uri = activeEditor.document.uri; - const diagnostics = uriStore[uri.fsPath] ?? []; + const diagnostics = formattedDiagnosticsStore.get(uri.fsPath) ?? []; const diagnostic = diagnostics.find((diagnostic) => diagnostic.range.contains(selection) ); diff --git a/docs/vscode-logs.md b/docs/vscode-logs.md new file mode 100644 index 0000000..9d2608c --- /dev/null +++ b/docs/vscode-logs.md @@ -0,0 +1,11 @@ +# Instructions to find and export the VS Code logs for Pretty TypeScript Errors + +1. Open the output window to the `Pretty TypeScript Errors` channel + ![Navigate to the `Pretty TypeScript Errors` output channel](./vscode-logs/1-output-window.png) +2. Set the log level of the `Pretty TypeScript Errors` output channel to `Trace` + ![Set the log level to `Trace`](vscode-logs/2-log-level.png) +3. **Reproduce your bug or error**, this should generate verbose logging output. +4. Either copy the output by selecting it or use one of the options in the menu shown: + ![Choose a method of copying or exporting the logs](vscode-logs/4-export-logs.png) +5. Either paste the output or add the logfile to the GitHub issue + ![Add the otuput to the GitHub issue](vscode-logs/5-add-logs-to-gh.png) diff --git a/docs/vscode-logs/1-output-window.png b/docs/vscode-logs/1-output-window.png new file mode 100644 index 0000000..80761d7 Binary files /dev/null and b/docs/vscode-logs/1-output-window.png differ diff --git a/docs/vscode-logs/2-log-level.png b/docs/vscode-logs/2-log-level.png new file mode 100644 index 0000000..fd8c875 Binary files /dev/null and b/docs/vscode-logs/2-log-level.png differ diff --git a/docs/vscode-logs/4-export-logs.png b/docs/vscode-logs/4-export-logs.png new file mode 100644 index 0000000..4c50706 Binary files /dev/null and b/docs/vscode-logs/4-export-logs.png differ diff --git a/docs/vscode-logs/5-add-logs-to-gh.png b/docs/vscode-logs/5-add-logs-to-gh.png new file mode 100644 index 0000000..e9b4f16 Binary files /dev/null and b/docs/vscode-logs/5-add-logs-to-gh.png differ diff --git a/eslint.config.mjs b/eslint.config.mjs index a05bce5..671c303 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -41,7 +41,7 @@ export default tseslint.config( "@typescript-eslint/no-unused-vars": "off", "@typescript-eslint/no-non-null-assertion": "off", curly: "warn", - eqeqeq: "warn", + eqeqeq: ["warn", { smart: true }], "no-throw-literal": "warn", semi: "off", },