From 2cba1378aaee29df5b6e66db1c2d77b9ab0ef924 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 16 Oct 2024 13:49:12 -0400 Subject: [PATCH 1/3] set up diagnostics service --- .../routes/tutorial/[...slug]/+page.svelte | 2 - packages/editor/src/lib/Editor.svelte | 39 +++++++++--------- packages/editor/src/lib/Workspace.svelte.ts | 30 ++++++++++++++ packages/editor/src/lib/diagnostics/index.ts | 40 +++++++++++++++++++ packages/editor/src/lib/diagnostics/worker.ts | 32 +++++++++++++++ packages/editor/tsconfig.json | 2 +- 6 files changed, 121 insertions(+), 24 deletions(-) create mode 100644 packages/editor/src/lib/diagnostics/index.ts create mode 100644 packages/editor/src/lib/diagnostics/worker.ts diff --git a/apps/svelte.dev/src/routes/tutorial/[...slug]/+page.svelte b/apps/svelte.dev/src/routes/tutorial/[...slug]/+page.svelte index a1c9d71b95..e1eeb99f96 100644 --- a/apps/svelte.dev/src/routes/tutorial/[...slug]/+page.svelte +++ b/apps/svelte.dev/src/routes/tutorial/[...slug]/+page.svelte @@ -316,8 +316,6 @@
{ skip_set_files = true; diff --git a/packages/editor/src/lib/Editor.svelte b/packages/editor/src/lib/Editor.svelte index f955e03365..3663b3a015 100644 --- a/packages/editor/src/lib/Editor.svelte +++ b/packages/editor/src/lib/Editor.svelte @@ -14,18 +14,15 @@ import { autocomplete_for_svelte } from '@sveltejs/site-kit/codemirror'; import type { Diagnostic } from '@codemirror/lint'; import { Workspace, type Item, type File } from './Workspace.svelte.js'; - import type { CompileError, Warning } from 'svelte/compiler'; import './codemirror.css'; interface Props { - errors: Record; - warnings: Record; workspace: Workspace; onchange: (file: File, contents: string) => void; autocomplete_filter?: (file: File) => boolean; } - let { errors, warnings, workspace, onchange, autocomplete_filter = () => true }: Props = $props(); + let { workspace, onchange, autocomplete_filter = () => true }: Props = $props(); let container: HTMLDivElement; @@ -171,25 +168,25 @@ const diagnostics: Diagnostic[] = []; - const error = null; // TODO should be `errors[workspace.selected_name]` but it's currently a Rollup plugin error... - const current_warnings = warnings[workspace.selected_name] || []; + const error = workspace.diagnostics[workspace.selected_name]?.error; + const current_warnings = workspace.diagnostics[workspace.selected_name]?.warnings ?? []; if (error) { - // diagnostics.push({ - // severity: 'error', - // from: error.position![0], - // to: error.position![1], - // message: error.message, - // renderMessage: () => { - // // TODO expose error codes, so we can link to docs in future - // const span = document.createElement('span'); - // span.innerHTML = `${error.message - // .replace(/&/g, '&') - // .replace(/$1`)}`; - // return span; - // } - // }); + diagnostics.push({ + severity: 'error', + from: error.position![0], + to: error.position![1], + message: error.message, + renderMessage: () => { + const span = document.createElement('span'); + span.innerHTML = `${error.message + .replace(/&/g, '&') + .replace(/$1`)} (${error.code})`; + + return span; + } + }); } for (const warning of current_warnings) { diff --git a/packages/editor/src/lib/Workspace.svelte.ts b/packages/editor/src/lib/Workspace.svelte.ts index f20fdd02cf..81dbe70df5 100644 --- a/packages/editor/src/lib/Workspace.svelte.ts +++ b/packages/editor/src/lib/Workspace.svelte.ts @@ -1,3 +1,6 @@ +import type { CompileError, Warning } from 'svelte/compiler'; +import { get_diagnostics } from './diagnostics'; + export interface File { type: 'file'; name: string; @@ -14,11 +17,18 @@ export interface Directory { export type Item = File | Directory; +export interface Diagnostics { + error: CompileError | null; + warnings: Warning[]; +} + export class Workspace { files = $state.raw([]); creating = $state.raw<{ parent: string; type: 'file' | 'directory' } | null>(null); selected_name = $state(null); + diagnostics = $state>({}); + #onupdate: (file: File) => void; #onreset: (items: Item[]) => void; @@ -37,6 +47,21 @@ export class Workspace { this.selected_name = selected_name; this.#onupdate = onupdate; this.#onreset = onreset; + + this.#reset_diagnostics(); + } + + #reset_diagnostics() { + this.diagnostics = {}; + + for (const file of this.files) { + if (file.type !== 'file') continue; + if (!/\.svelte(\.|$)/.test(file.name)) continue; + + get_diagnostics(file).then((diagnostics) => { + this.diagnostics[file.name] = diagnostics; + }); + } } get selected_file() { @@ -55,6 +80,10 @@ export class Workspace { return old; }); + get_diagnostics(file).then((diagnostics) => { + this.diagnostics[file.name] = diagnostics; + }); + this.#onupdate(file); } @@ -67,5 +96,6 @@ export class Workspace { this.files = new_files; this.#onreset(new_files); + this.#reset_diagnostics(); } } diff --git a/packages/editor/src/lib/diagnostics/index.ts b/packages/editor/src/lib/diagnostics/index.ts new file mode 100644 index 0000000000..883b672afc --- /dev/null +++ b/packages/editor/src/lib/diagnostics/index.ts @@ -0,0 +1,40 @@ +import { BROWSER } from 'esm-env'; +import DiagnosticsWorker from './worker?worker'; +import type { Diagnostics, File } from '../Workspace.svelte'; + +const callbacks = new Map void>(); + +let worker: Worker; + +let uid = 1; + +if (BROWSER) { + worker = new DiagnosticsWorker(); + + worker.addEventListener('message', (event) => { + const callback = callbacks.get(event.data.id); + + if (callback) { + callback(event.data.payload); + callbacks.delete(event.data.id); + } + }); +} + +export function get_diagnostics(file: File): Promise { + if (!BROWSER) { + // TODO this is a bit janky + return Promise.resolve({ + error: null, + warnings: [] + }); + } + + let id = uid++; + + worker.postMessage({ id, file }); + + return new Promise((fulfil) => { + callbacks.set(id, fulfil); + }); +} diff --git a/packages/editor/src/lib/diagnostics/worker.ts b/packages/editor/src/lib/diagnostics/worker.ts new file mode 100644 index 0000000000..e197959dd7 --- /dev/null +++ b/packages/editor/src/lib/diagnostics/worker.ts @@ -0,0 +1,32 @@ +import { compile, compileModule } from 'svelte/compiler'; +import type { File } from '../Workspace.svelte'; + +addEventListener('message', (event) => { + const { id, file } = event.data as { id: number; file: File }; + + const fn = file.name.endsWith('.svelte') ? compile : compileModule; + + try { + const result = fn(file.contents, { + filename: file.name + }); + + postMessage({ + id, + payload: { + error: null, + // @ts-expect-error https://github.com/sveltejs/svelte/issues/13628 + warnings: result.warnings.map((w) => ({ message: w.message, ...w })) + } + }); + } catch (e) { + postMessage({ + id, + payload: { + // @ts-expect-error + error: { message: e.message, ...e }, + warnings: [] + } + }); + } +}); diff --git a/packages/editor/tsconfig.json b/packages/editor/tsconfig.json index d4a5412e42..c4a014bac3 100644 --- a/packages/editor/tsconfig.json +++ b/packages/editor/tsconfig.json @@ -9,7 +9,7 @@ "skipLibCheck": true, "sourceMap": true, "strict": true, - "module": "NodeNext", + "module": "preserve", "moduleResolution": "bundler" } } From f75fb6c03f9bed4bc85e3b157da77b08a2da08a3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 16 Oct 2024 13:54:15 -0400 Subject: [PATCH 2/3] remove diagnostics from adapters --- .../tutorial/adapters/rollup/index.svelte.ts | 18 -------- .../adapters/webcontainer/index.svelte.ts | 46 +------------------ .../tutorial/[...slug]/adapter.svelte.ts | 4 -- 3 files changed, 1 insertion(+), 67 deletions(-) diff --git a/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts b/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts index a527605c67..6987f618a4 100644 --- a/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts +++ b/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts @@ -3,7 +3,6 @@ import Bundler from '@sveltejs/repl/bundler'; import * as yootils from 'yootils'; import type { Adapter } from '$lib/tutorial'; import type { File, Item } from 'editor'; -import type { CompileError, Warning } from 'svelte/compiler'; /** Rollup bundler singleton */ let bundler: Bundler; @@ -11,8 +10,6 @@ let bundler: Bundler; export const state = new (class RollupState { progress = $state.raw({ value: 0, text: 'initialising' }); bundle = $state.raw(null); - errors = $state.raw>(); - warnings = $state.raw>({}); })(); /** @@ -55,21 +52,6 @@ export async function create(): Promise { ); state.bundle = result; - - // TODO this approach is insufficient — we need to get diagnostics for - // individual files, not just the bundle as a whole - state.errors = {}; - state.warnings = {}; - - if (result.error) { - const file = '/src/lib/' + result.error.filename; - state.errors[file] = result.error; - } - - for (const warning of result?.warnings ?? []) { - const file = '/src/lib/' + warning.filename; - (state.warnings[file] ??= []).push(warning); - } } const q = yootils.queue(1); diff --git a/apps/svelte.dev/src/lib/tutorial/adapters/webcontainer/index.svelte.ts b/apps/svelte.dev/src/lib/tutorial/adapters/webcontainer/index.svelte.ts index 29e13709e1..c1cec0fece 100644 --- a/apps/svelte.dev/src/lib/tutorial/adapters/webcontainer/index.svelte.ts +++ b/apps/svelte.dev/src/lib/tutorial/adapters/webcontainer/index.svelte.ts @@ -8,7 +8,6 @@ import { escape_html } from '../../../utils/escape.js'; import { ready } from '../common/index.js'; import type { Adapter } from '$lib/tutorial'; import type { Item, File } from 'editor'; -import type { CompileError, Warning } from 'svelte/compiler'; const converter = new AnsiToHtml({ fg: 'var(--sk-text-3)' @@ -22,8 +21,6 @@ export const state = new (class WCState { base = $state.raw(null); error = $state.raw(null); logs = $state.raw([]); - errors = $state.raw>({}); - warnings = $state.raw>({}); })(); export async function create(): Promise { @@ -49,14 +46,6 @@ export async function create(): Promise { } }); - let warnings: Record = {}; - let timeout: any; - - function schedule_to_update_warning(msec: number) { - clearTimeout(timeout); - timeout = setTimeout(() => (state.warnings = { ...warnings }), msec); - } - const log_stream = () => new WritableStream({ write(chunk) { @@ -64,25 +53,7 @@ export async function create(): Promise { // clear screen state.logs = []; } else if (chunk?.startsWith('svelte:warnings:')) { - const warn: Warning = JSON.parse(chunk.slice(16)); - const filename = (warn.filename!.startsWith('/') ? warn.filename : '/' + warn.filename)!; - const current = warnings[filename]; - - if (!current) { - warnings[filename] = [warn]; - // the exact same warning may be given multiple times in a row - } else if ( - !current.some( - (s) => - s.code === warn.code && - s.position![0] === warn.position![0] && - s.position![1] === warn.position![1] - ) - ) { - current.push(warn); - } - - schedule_to_update_warning(100); + // TODO when does this happen? } else { const log = converter.toHtml(escape_html(chunk)).replace(/\n/g, '
'); state.logs = [...state.logs, log]; @@ -181,17 +152,6 @@ export async function create(): Promise { ...force_delete ]; - // initialize warnings of written files - to_write - .filter((stub) => stub.type === 'file' && warnings[stub.name]) - .forEach((stub) => (warnings[stub.name] = [])); - // remove warnings of deleted files - to_delete - .filter((stubname) => warnings[stubname]) - .forEach((stubname) => delete warnings[stubname]); - - state.warnings = { ...warnings }; - current_stubs = stubs_to_map(stubs); // For some reason, server-ready is fired again when the vite dev server is restarted. @@ -258,10 +218,6 @@ export async function create(): Promise { tree[basename] = to_file(file); - // initialize warnings of this file - warnings[file.name] = []; - schedule_to_update_warning(100); - await vm.mount(root); if (will_restart) await wait_for_restart_vite(); diff --git a/apps/svelte.dev/src/routes/tutorial/[...slug]/adapter.svelte.ts b/apps/svelte.dev/src/routes/tutorial/[...slug]/adapter.svelte.ts index ab2d84ad77..350f8c039e 100644 --- a/apps/svelte.dev/src/routes/tutorial/[...slug]/adapter.svelte.ts +++ b/apps/svelte.dev/src/routes/tutorial/[...slug]/adapter.svelte.ts @@ -31,10 +31,6 @@ export const adapter_state = new (class { status: 'initialising' } ); - - /** Diagnostics */ - errors = $derived((use_rollup ? rollup_state.errors : wc_state.errors) || {}); - warnings = $derived((use_rollup ? rollup_state.warnings : wc_state.warnings) || {}); })(); if (browser) { From d4e566be7aeb8468dee6c124949c234baf90aaed Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 16 Oct 2024 14:00:04 -0400 Subject: [PATCH 3/3] tidy up --- .../scripts/create-tutorial-zip/common/svelte.config.js | 9 --------- .../src/lib/tutorial/adapters/rollup/index.svelte.ts | 4 +--- packages/editor/src/lib/diagnostics/worker.ts | 2 ++ 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/apps/svelte.dev/scripts/create-tutorial-zip/common/svelte.config.js b/apps/svelte.dev/scripts/create-tutorial-zip/common/svelte.config.js index d2deb768bc..29ce35b4be 100644 --- a/apps/svelte.dev/scripts/create-tutorial-zip/common/svelte.config.js +++ b/apps/svelte.dev/scripts/create-tutorial-zip/common/svelte.config.js @@ -5,15 +5,6 @@ const config = { // Don't do this in your own apps unless you know what you're doing! // See https://kit.svelte.dev/docs/configuration#csrf for more info. csrf: false - }, - - vitePlugin: { - // This enables compile-time warnings to be - // visible in the learn.svelte.dev editor - onwarn: (warning, defaultHandler) => { - console.log('svelte:warnings:%s', JSON.stringify(warning)); - defaultHandler(warning); - } } }; diff --git a/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts b/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts index 6987f618a4..16b08844e8 100644 --- a/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts +++ b/apps/svelte.dev/src/lib/tutorial/adapters/rollup/index.svelte.ts @@ -40,7 +40,7 @@ export async function create(): Promise { let current_stubs = stubs_to_map([]); async function compile() { - const result = await bundler.bundle( + state.bundle = await bundler.bundle( [...current_stubs.values()] // TODO we can probably remove all the SvelteKit specific stuff from the tutorial content once this settles down .filter((f): f is File => f.name.startsWith('/src/lib/') && f.type === 'file') @@ -50,8 +50,6 @@ export async function create(): Promise { type: f.name.split('.').pop() ?? 'svelte' })) ); - - state.bundle = result; } const q = yootils.queue(1); diff --git a/packages/editor/src/lib/diagnostics/worker.ts b/packages/editor/src/lib/diagnostics/worker.ts index e197959dd7..baeaf30ad0 100644 --- a/packages/editor/src/lib/diagnostics/worker.ts +++ b/packages/editor/src/lib/diagnostics/worker.ts @@ -1,6 +1,8 @@ import { compile, compileModule } from 'svelte/compiler'; import type { File } from '../Workspace.svelte'; +// TODO need to handle Svelte 3/4 for playground + addEventListener('message', (event) => { const { id, file } = event.data as { id: number; file: File };