diff --git a/src/internal/build.ts b/src/internal/build.ts index ac00b756..c347ffba 100644 --- a/src/internal/build.ts +++ b/src/internal/build.ts @@ -6,16 +6,13 @@ import {Deferred} from '../shared/deferred.js'; -import { - SampleFile, - BuildOutput, - FileBuildOutput, - DiagnosticBuildOutput, +import type { + File, + FileDiagnostic, + Diagnostic, HttpError, + BuildResult, } from '../shared/worker-api.js'; -import {Diagnostic} from 'vscode-languageserver-protocol'; - -const unreachable = (n: never) => n; type State = 'active' | 'done' | 'cancelled'; @@ -36,7 +33,7 @@ export class PlaygroundBuild { diagnostics = new Map(); private _state: State = 'active'; private _stateChange = new Deferred(); - private _files = new Map>(); + private _files = new Map>(); private _diagnosticsCallback: () => void; private _diagnosticsDebounceId: number | undefined; @@ -44,7 +41,7 @@ export class PlaygroundBuild { * @param diagnosticsCallback Function that will be invoked when one or more * new diagnostics have been received. Fires at most once per animation frame. */ - constructor(diagnosticsCallback: () => void) { + constructor({diagnosticsCallback}: {diagnosticsCallback: () => void}) { this._diagnosticsCallback = diagnosticsCallback; } @@ -79,10 +76,14 @@ export class PlaygroundBuild { * received before the build is completed or cancelled, this promise will be * rejected. */ - async getFile(name: string): Promise { + async getFile(name: string): Promise { let deferred = this._files.get(name); if (deferred === undefined) { if (this._state === 'done') { + // TODO (justinfagnani): If the file is a package dependency (in + // 'node_modules/'), get the file from the TypeScript worker here + // rather than assuming that it is present in the files cache. + // Let the worker handle the error if the file is not found. return errorNotFound; } else if (this._state === 'cancelled') { return errorCancelled; @@ -94,24 +95,25 @@ export class PlaygroundBuild { } /** - * Handle a worker build output. + * Handle a worker build result. */ - onOutput(output: BuildOutput) { + onResult(output: BuildResult) { if (this._state !== 'active') { return; } - if (output.kind === 'file') { - this._onFile(output); - } else if (output.kind === 'diagnostic') { - this._onDiagnostic(output); - } else if (output.kind === 'done') { - this._onDone(); - } else { - throw new Error( - `Unexpected BuildOutput kind: ${ - (unreachable(output) as BuildOutput).kind - }` - ); + for (const file of output.files) { + this._onFile(file); + } + for (const fileDiagnostic of output.diagnostics) { + this._onDiagnostic(fileDiagnostic); + } + } + + onSemanticDiagnostics(semanticDiagnostics?: Array) { + if (semanticDiagnostics !== undefined) { + for (const fileDiagnostic of semanticDiagnostics) { + this._onDiagnostic(fileDiagnostic); + } } } @@ -121,22 +123,22 @@ export class PlaygroundBuild { this._stateChange = new Deferred(); } - private _onFile(output: FileBuildOutput) { - let deferred = this._files.get(output.file.name); + private _onFile(file: File) { + let deferred = this._files.get(file.name); if (deferred === undefined) { deferred = new Deferred(); - this._files.set(output.file.name, deferred); + this._files.set(file.name, deferred); } - deferred.resolve(output.file); + deferred.resolve(file); } - private _onDiagnostic(output: DiagnosticBuildOutput) { - let arr = this.diagnostics.get(output.filename); + private _onDiagnostic(fileDiagnostic: FileDiagnostic) { + let arr = this.diagnostics.get(fileDiagnostic.filename); if (arr === undefined) { arr = []; - this.diagnostics.set(output.filename, arr); + this.diagnostics.set(fileDiagnostic.filename, arr); } - arr.push(output.diagnostic); + arr.push(fileDiagnostic.diagnostic); if (this._diagnosticsDebounceId === undefined) { this._diagnosticsDebounceId = requestAnimationFrame(() => { if (this._state !== 'cancelled') { @@ -147,7 +149,13 @@ export class PlaygroundBuild { } } - private _onDone() { + /** + * Completes a build. Must be called after onResult() and + * onSemanticDiagnostics(). + * + * TODO (justinfagnani): do this automatically? + */ + onDone() { this._errorPendingFileRequests(errorNotFound); this._changeState('done'); } diff --git a/src/playground-code-editor.ts b/src/playground-code-editor.ts index 4010e076..902851c6 100644 --- a/src/playground-code-editor.ts +++ b/src/playground-code-editor.ts @@ -19,7 +19,6 @@ import {ifDefined} from 'lit/directives/if-defined.js'; import {CodeMirror} from './internal/codemirror.js'; import playgroundStyles from './playground-styles.js'; import './internal/overlay.js'; -import {Diagnostic} from 'vscode-languageserver-protocol'; import { Doc, Editor, @@ -35,6 +34,7 @@ import { EditorPosition, EditorToken, CodeEditorChangeData, + type Diagnostic, } from './shared/worker-api.js'; // TODO(aomarks) Could we upstream this to lit-element? It adds much stricter diff --git a/src/playground-project.ts b/src/playground-project.ts index b5e40fad..9746cf8a 100644 --- a/src/playground-project.ts +++ b/src/playground-project.ts @@ -9,19 +9,21 @@ import {customElement, property, query, state} from 'lit/decorators.js'; import {wrap, Remote, proxy} from 'comlink'; import { - SampleFile, - ServiceWorkerAPI, - ProjectManifest, - PlaygroundMessage, - WorkerAPI, + type SampleFile, + type ServiceWorkerAPI, + type ProjectManifest, + type PlaygroundMessage, + type WorkerAPI, CONFIGURE_PROXY, CONNECT_PROJECT_TO_SW, ACKNOWLEDGE_SW_CONNECTION, - ModuleImportMap, - HttpError, + type ModuleImportMap, + type HttpError, UPDATE_SERVICE_WORKER, - CodeEditorChangeData, - CompletionInfoWithDetails, + type CodeEditorChangeData, + type CompletionInfoWithDetails, + type Diagnostic, + FileDiagnostic, } from './shared/worker-api.js'; import { getRandomString, @@ -37,8 +39,6 @@ import {npmVersion, serviceWorkerHash} from './shared/version.js'; import {Deferred} from './shared/deferred.js'; import {PlaygroundBuild} from './internal/build.js'; -import {Diagnostic} from 'vscode-languageserver-protocol'; - // Each has a unique session ID used to scope requests from // the preview iframes. const sessions = new Set(); @@ -559,26 +559,30 @@ export class PlaygroundProject extends LitElement { */ async save() { this._build?.cancel(); - const build = new PlaygroundBuild(() => { - this.dispatchEvent(new CustomEvent('diagnosticsChanged')); - }); - this._build = build; - this.dispatchEvent(new CustomEvent('compileStart')); const workerApi = await this._deferredTypeScriptWorkerApi.promise; + const build = (this._build = new PlaygroundBuild({ + diagnosticsCallback: () => { + this.dispatchEvent(new CustomEvent('diagnosticsChanged')); + }, + })); + this.dispatchEvent(new CustomEvent('compileStart')); if (build.state() !== 'active') { return; } - /* eslint-disable @typescript-eslint/no-floating-promises */ - workerApi.compileProject( + const receivedSemanticDiagnostics = new Deferred(); + const result = await workerApi.compileProject( this._files ?? [], - {importMap: this._importMap}, - proxy((result) => build.onOutput(result)) + { + importMap: this._importMap, + }, + proxy((diagnostics?: Array) => { + build.onSemanticDiagnostics(diagnostics); + receivedSemanticDiagnostics.resolve(); + }) ); - /* eslint-enable @typescript-eslint/no-floating-promises */ - await build.stateChange; - if (build.state() !== 'done') { - return; - } + build.onResult(result); + await receivedSemanticDiagnostics.promise; + build.onDone(); this.dispatchEvent(new CustomEvent('compileDone')); } diff --git a/src/shared/worker-api.ts b/src/shared/worker-api.ts index 0291110e..711d2fcd 100755 --- a/src/shared/worker-api.ts +++ b/src/shared/worker-api.ts @@ -5,7 +5,8 @@ */ import {CompletionEntry, CompletionInfo, WithMetadata} from 'typescript'; -import {Diagnostic} from 'vscode-languageserver-protocol'; +import type {Diagnostic} from 'vscode-languageserver-protocol'; +export type {Diagnostic} from 'vscode-languageserver-protocol'; /** * Sent from the project to the proxy, with configuration and a port for further @@ -123,10 +124,10 @@ export interface EditorCompletionDetails { export interface WorkerAPI { compileProject( - files: Array, + files: Array, config: WorkerConfig, - emit: (result: BuildOutput) => void - ): Promise; + onSemanticDiagnostics?: (diagnostics?: Array) => void + ): Promise; getCompletions( filename: string, fileContent: string, @@ -142,6 +143,15 @@ export interface WorkerAPI { ): Promise; } +export interface File { + /** Filename. */ + name: string; + /** File contents. */ + content: string; + /** MIME type. */ + contentType?: string; +} + export interface HttpError { status: number; body: string; @@ -151,15 +161,9 @@ export interface FileAPI { getFile(name: string): Promise; } -export interface SampleFile { - /** Filename. */ - name: string; +export interface SampleFile extends File { /** Optional display label. */ label?: string; - /** File contents. */ - content: string; - /** MIME type. */ - contentType?: string; /** Don't display in tab bar. */ hidden?: boolean; /** Whether the file should be selected when loaded */ @@ -213,19 +217,35 @@ export interface CompletionInfoWithDetails entries: CompletionEntryWithDetails[]; } -export type BuildOutput = FileBuildOutput | DiagnosticBuildOutput | DoneOutput; - -export type FileBuildOutput = { - kind: 'file'; - file: SampleFile; -}; - -export type DiagnosticBuildOutput = { - kind: 'diagnostic'; +export interface FileDiagnostic { filename: string; diagnostic: Diagnostic; -}; +} -export type DoneOutput = { - kind: 'done'; -}; +export interface BuildResult { + files: Array; + diagnostics: Array; + semanticDiagnostics?: Promise>; +} + +export interface FileResult { + file?: File; + diagnostics: Array; +} + +// export type BuildOutput = FileBuildOutput | DiagnosticBuildOutput | DoneOutput; + +// export type FileBuildOutput = { +// kind: 'file'; +// file: SampleFile; +// }; + +// export type DiagnosticBuildOutput = { +// kind: 'diagnostic'; +// filename: string; +// diagnostic: Diagnostic; +// }; + +// export type DoneOutput = { +// kind: 'done'; +// }; diff --git a/src/test/bare-module-worker_test.ts b/src/test/bare-module-worker_test.ts index 4a521483..d1971d63 100644 --- a/src/test/bare-module-worker_test.ts +++ b/src/test/bare-module-worker_test.ts @@ -6,11 +6,7 @@ import {checkTransform} from './worker-test-util.js'; -import { - BuildOutput, - ModuleImportMap, - SampleFile, -} from '../shared/worker-api.js'; +import {BuildResult, ModuleImportMap, File} from '../shared/worker-api.js'; import {CdnData} from './fake-cdn-plugin.js'; const browser = (() => { @@ -29,14 +25,17 @@ const browser = (() => { suite('bare module worker', () => { test('empty project', async () => { - const files: SampleFile[] = []; + const files: File[] = []; const cdn: CdnData = {}; - const expected: BuildOutput[] = []; + const expected: BuildResult = { + files: [], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('one simple static import', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/index.js";', @@ -55,28 +54,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: 'index', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('multiple static and dynamic imports', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: ` @@ -105,10 +101,9 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: ` import {a} from "./node_modules/foo@1.0.0/index.js"; @@ -117,10 +112,7 @@ suite('bare module worker', () => { const {d} = import('./node_modules/foo@1.0.0/index.js'); `, }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: ` export const a = 'a'; @@ -130,13 +122,14 @@ suite('bare module worker', () => { `, contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('namespaced NPM package', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "@foo/bar/index.js";', @@ -155,28 +148,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/@foo/bar@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/@foo/bar@1.0.0/index.js', content: 'index', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('default module uses package.json "module" field as 1st choice', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo";', @@ -207,28 +197,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/module.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/module.js', content: 'module', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('default module uses package.json "main" field as 2nd choice', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo";', @@ -258,28 +245,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/main.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/main.js', content: 'main', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('default module uses "index.js" field as 3rd choice', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo";', @@ -304,28 +288,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: 'index', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('project file in nested directory', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'some/sub/dir/index.js', content: 'import "foo/index.js";', @@ -344,28 +325,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'some/sub/dir/index.js', content: 'import "../../../node_modules/foo@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: 'index', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('import chain in dependency', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/a.js";', @@ -390,44 +368,35 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/a.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/a.js', content: 'import "./b.js";', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/b.js', content: 'import "./c.js";', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/c.js', content: 'c', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('circular import in dependency', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/a.js";', @@ -449,36 +418,30 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/a.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/a.js', content: 'import "./b.js";', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/b.js', content: 'import "./a.js";', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test("version specified in local project's package.json", async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/index.js";', @@ -521,25 +484,18 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@2.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@2.0.0/index.js', content: 'foo2', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'package.json', content: ` { @@ -549,13 +505,14 @@ suite('bare module worker', () => { } `, }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test("version specified in dependency's package.json", async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/index.js";', @@ -606,36 +563,30 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: 'import "../bar@2.0.0/index.js";', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/bar@2.0.0/index.js', content: 'bar2', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('extension-less import in project file', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/bar";', @@ -654,28 +605,25 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/bar.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/bar.js', contentType: 'text/javascript; charset=utf-8', content: 'bar', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('extension-less import in dependency', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/a.js";', @@ -697,74 +645,68 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/a.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/a.js', contentType: 'text/javascript; charset=utf-8', content: 'import "./b.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/b.js', contentType: 'text/javascript; charset=utf-8', content: 'b', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('non-existent dependency', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "non-existent/index.js";', }, ]; const cdn: CdnData = {}; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "non-existent/index.js";', }, - }, - { - diagnostic: { - message: - 'Could not resolve module "non-existent/index.js": CDN HTTP 404 error (/non-existent@latest/package.json): Not Found', - range: { - end: { - character: 29, - line: 0, - }, - start: { - character: 8, - line: 0, + ], + diagnostics: [ + { + diagnostic: { + message: + 'Could not resolve module "non-existent/index.js": CDN HTTP 404 error (/non-existent@latest/package.json): Not Found', + range: { + end: { + character: 29, + line: 0, + }, + start: { + character: 8, + line: 0, + }, }, }, + filename: 'index.js', }, - filename: 'index.js', - kind: 'diagnostic', - }, - ]; + ], + }; await checkTransform(files, expected, {}, cdn); }); test('invalid package.json', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo/index.js";', @@ -789,60 +731,54 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "./node_modules/foo@1.0.0/index.js";', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/index.js', content: 'foo1', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'package.json', content: `{ "dependencies": XXX }`, }, - }, - { - diagnostic: { - message: - 'Invalid package.json: ' + - { - chrome: - 'SyntaxError: Unexpected token \'X\', ..."dencies": XXX\n "... is not valid JSON', - firefox: - 'SyntaxError: JSON.parse: unexpected character at line 2 column 28 of the JSON data', - safari: - 'SyntaxError: JSON Parse error: Unexpected identifier "XXX"', - }[browser], - range: { - start: - browser === 'safari' || browser === 'chrome' - ? {line: 0, character: 0} - : {line: 1, character: 27}, - end: {line: 2, character: 10}, + ], + diagnostics: [ + { + diagnostic: { + message: + 'Invalid package.json: ' + + { + chrome: + 'SyntaxError: Unexpected token \'X\', ..."dencies": XXX\n "... is not valid JSON', + firefox: + 'SyntaxError: JSON.parse: unexpected character at line 2 column 28 of the JSON data', + safari: + 'SyntaxError: JSON Parse error: Unexpected identifier "XXX"', + }[browser], + range: { + start: + browser === 'safari' || browser === 'chrome' + ? {line: 0, character: 0} + : {line: 1, character: 27}, + end: {line: 2, character: 10}, + }, }, + filename: 'package.json', }, - filename: 'package.json', - kind: 'diagnostic', - }, - ]; + ], + }; await checkTransform(files, expected, {}, cdn); }); test('use import map', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'import "foo";', @@ -866,20 +802,20 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import "http://example.com/foo";', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, importMap, cdn); }); test('prefers "import" condition exports', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: ` @@ -932,39 +868,33 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: ` import './node_modules/foo@1.0.0/esm/index.js'; import './node_modules/foo@1.0.0/esm/bar.js'; `, }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/esm/index.js', content: 'ESM', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/esm/bar.js', content: 'ESM', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('prefers "development" condition exports', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: ` @@ -1009,39 +939,33 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: ` import './node_modules/foo@1.0.0/development/index.js'; import './node_modules/foo@1.0.0/development/bar.js'; `, }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/development/index.js', content: 'DEV', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/development/bar.js', content: 'DEV', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('prefers "import" + "browser" condition exports over default', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: ` @@ -1091,39 +1015,33 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: ` import './node_modules/foo@1.0.0/browser/index.js'; import './node_modules/foo@1.0.0/browser/bar.js'; `, }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/browser/index.js', content: 'BROWSER', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@1.0.0/browser/bar.js', content: 'BROWSER', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); test('@lion/button style import', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: ` @@ -1158,33 +1076,27 @@ suite('bare module worker', () => { }, }, }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: ` import './node_modules/@lion/button@0.14.3/define.js'; `, }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/@lion/button@0.14.3/define.js', content: "import './lion-button.js';", contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/@lion/button@0.14.3/lion-button.js', content: 'customElements.define(...);', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected, {}, cdn); }); }); diff --git a/src/test/typescript-worker_test.ts b/src/test/typescript-worker_test.ts index 7d2f013b..d7a246d6 100644 --- a/src/test/typescript-worker_test.ts +++ b/src/test/typescript-worker_test.ts @@ -6,99 +6,102 @@ import {checkTransform} from './worker-test-util.js'; -import {BuildOutput, SampleFile} from '../shared/worker-api.js'; +import {BuildResult, File} from '../shared/worker-api.js'; import {CdnData} from './fake-cdn-plugin.js'; suite('typescript builder', () => { test('empty project', async () => { - const files: SampleFile[] = []; - const expected: BuildOutput[] = []; + const files: File[] = []; + const expected: BuildResult = { + files: [], + diagnostics: [], + }; await checkTransform(files, expected); }); test('compiles ts file to js', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.ts', content: 'export const foo: number = 3;', }, ]; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'export const foo = 3;\n', contentType: 'text/javascript', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected); }); test('ignores js file', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.js', content: 'export const foo: number = 3;', }, ]; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'export const foo: number = 3;', }, - }, - ]; + ], + diagnostics: [], + }; await checkTransform(files, expected); }); test('emits syntax error', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.ts', content: ':', }, ]; - const expected: BuildOutput[] = [ - { - diagnostic: { - code: 1128, - message: 'Declaration or statement expected.', - range: { - end: { - character: 1, - line: 0, - }, - start: { - character: 0, - line: 0, - }, - }, - severity: 1, - source: 'typescript', - }, - filename: 'index.ts', - kind: 'diagnostic', - }, - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', // TODO(aomarks) This should probably return a 400 error instead of an // empty but valid file. content: 'export {};\n', contentType: 'text/javascript', }, - }, - ]; + ], + diagnostics: [ + { + filename: 'index.ts', + diagnostic: { + code: 1128, + message: 'Declaration or statement expected.', + range: { + end: { + character: 1, + line: 0, + }, + start: { + character: 0, + line: 0, + }, + }, + severity: 1, + source: 'typescript', + }, + }, + ], + }; await checkTransform(files, expected); }); test('emits local semantic error', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.ts', content: ` @@ -107,41 +110,42 @@ suite('typescript builder', () => { `, }, ]; - const expected: BuildOutput[] = [ - { - diagnostic: { - code: 2322, - message: "Type 'string' is not assignable to type 'number'.", - range: { - end: { - character: 13, - line: 2, - }, - start: { - character: 10, - line: 2, - }, - }, - severity: 1, - source: 'typescript', - }, - filename: 'index.ts', - kind: 'diagnostic', - }, - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'let foo = 3;\n' + 'foo = "foo";\nexport {};\n', contentType: 'text/javascript', }, - }, - ]; + ], + semanticDiagnostics: Promise.resolve([ + { + fileName: 'index.ts', + diagnostic: { + code: 2322, + message: "Type 'string' is not assignable to type 'number'.", + range: { + end: { + character: 13, + line: 2, + }, + start: { + character: 10, + line: 2, + }, + }, + severity: 1, + source: 'typescript', + }, + }, + ]), + diagnostics: [], + }; await checkTransform(files, expected); }); test('emits semantic error from bare module', async () => { - const files: SampleFile[] = [ + const files: File[] = [ { name: 'index.ts', content: ` @@ -177,551 +181,546 @@ suite('typescript builder', () => { }, }, }; - const expected: BuildOutput[] = [ - { - diagnostic: { - code: 2345, - message: - "Argument of type 'number' is not assignable to parameter of type 'string'.", - range: { - end: { - character: 19, - line: 3, - }, - start: { - character: 16, - line: 3, - }, - }, - severity: 1, - source: 'typescript', - }, - filename: 'index.ts', - kind: 'diagnostic', - }, - { - diagnostic: { - code: 2345, - message: - "Argument of type 'string' is not assignable to parameter of type 'number'.", - range: { - end: { - character: 21, - line: 4, - }, - start: { - character: 16, - line: 4, - }, - }, - severity: 1, - source: 'typescript', - }, - filename: 'index.ts', - kind: 'diagnostic', - }, - { - kind: 'file', - file: { + const expected: BuildResult = { + files: [ + { name: 'index.js', content: 'import { strFn } from "./node_modules/foo@2.0.0/index.js";\nimport { numFn } from "./node_modules/foo@2.0.0/bar.js";\nstrFn(123);\nnumFn("str");\n', contentType: 'text/javascript', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@2.0.0/index.js', content: 'export const strFn = (s) => s;', contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { + { name: 'node_modules/foo@2.0.0/bar.js', content: 'export const numFn = (n) => n;', contentType: 'text/javascript; charset=utf-8', }, - }, - ]; - await checkTransform(files, expected, {}, cdn); - }); + ], - test('respects package.json dependency for semantic errors', async () => { - const files: SampleFile[] = [ - { - name: 'index.ts', - content: ` - import {foo} from "foo"; - foo(123); - `, - }, - { - name: 'package.json', - content: `{ - "dependencies": { - "foo": "^1.0.0" - } - }`, - }, - ]; - const cdn: CdnData = { - foo: { - // foo takes a string in 1.0.0, and a number in 2.0.0. We should expect - // an error because we depend on 1.0.0. - versions: { - '1.0.0': { - files: { - 'index.js': { - content: 'export const foo = (s) => s;', - }, - 'index.d.ts': { - content: ` - import type {t} from './type.js'; - export declare const foo: (s: t) => t; - `, + semanticDiagnostics: Promise.resolve([ + { + fileName: 'index.ts', + diagnostic: { + code: 2345, + message: + "Argument of type 'number' is not assignable to parameter of type 'string'.", + range: { + end: { + character: 19, + line: 3, }, - 'type.d.ts': { - content: `export type t = string;`, + start: { + character: 16, + line: 3, }, }, + severity: 1, + source: 'typescript', }, - '2.0.0': { - files: { - 'index.js': { - content: 'export const foo = (n) => n;', - }, - 'index.d.ts': { - content: 'export declare const foo: (n: number) => number;', - }, - }, - }, - }, - }, - }; - const expected: BuildOutput[] = [ - { - diagnostic: { - code: 2345, - message: - "Argument of type 'number' is not assignable to parameter of type 'string'.", - range: { - end: { - character: 17, - line: 2, - }, - start: { - character: 14, - line: 2, - }, - }, - severity: 1, - source: 'typescript', - }, - filename: 'index.ts', - kind: 'diagnostic', - }, - { - kind: 'file', - file: { - name: 'index.js', - content: - 'import { foo } from "./node_modules/foo@1.0.0/index.js";\nfoo(123);\n', - contentType: 'text/javascript', - }, - }, - { - kind: 'file', - file: { - name: 'node_modules/foo@1.0.0/index.js', - content: 'export const foo = (s) => s;', - contentType: 'text/javascript; charset=utf-8', }, - }, - { - kind: 'file', - file: { - name: 'package.json', - content: `{ - "dependencies": { - "foo": "^1.0.0" - } - }`, - }, - }, - ]; - await checkTransform(files, expected, {}, cdn); - }); - - test('prefers typings from: typings, types, main, index.d.ts', async () => { - const wrong = { - content: 'export type StringType = number;', - }; - const files: SampleFile[] = [ - { - name: 'index.ts', - content: ` - import {StringType} from "a"; - export const str: StringType = "foo"; - `, - }, - ]; - const cdn: CdnData = { - a: { - versions: { - '1.0.0': { - files: { - 'package.json': { - content: `{ - "typings": "typings.d.ts", - "types": "types.d.ts", - "main": "main.js" - }`, + { + fileName: 'index.ts', + diagnostic: { + code: 2345, + message: + "Argument of type 'string' is not assignable to parameter of type 'number'.", + range: { + end: { + character: 21, + line: 4, }, - 'typings.d.ts': { - content: 'export * from "b";', + start: { + character: 16, + line: 4, }, - 'types.d.ts': wrong, - 'main.d.ts': wrong, - 'index.d.ts': wrong, }, + severity: 1, + source: 'typescript', }, }, - }, - b: { - versions: { - '1.0.0': { - files: { - 'package.json': { - content: `{ - "types": "types.d.ts", - "main": "main.js" - }`, - }, - 'typings.d.ts': wrong, - 'types.d.ts': { - content: 'export * from "c";', - }, - 'main.d.ts': wrong, - 'index.d.ts': wrong, - }, - }, - }, - }, - c: { - versions: { - '1.0.0': { - files: { - 'package.json': { - content: `{ - "main": "main.js" - }`, - }, - 'typings.d.ts': wrong, - 'types.d.ts': wrong, - 'main.d.ts': { - content: 'export * from "d";', - }, - 'index.d.ts': wrong, - }, - }, - }, - }, - d: { - versions: { - '1.0.0': { - files: { - 'package.json': { - content: `{}`, - }, - 'typings.d.ts': wrong, - 'types.d.ts': wrong, - 'main.d.ts': wrong, - 'index.d.ts': { - content: 'export type StringType = string;', - }, - }, - }, - }, - }, + ]), + diagnostics: [], }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { - name: 'index.js', - content: `export const str = "foo";\n`, - contentType: 'text/javascript', - }, - }, - ]; await checkTransform(files, expected, {}, cdn); }); - test('handles multiple versions', async () => { - const files: SampleFile[] = [ - { - name: 'index.ts', - content: ` - import {num} from "foo"; - import {str} from "str-or-num"; - const a: string = str; - const b: number = num - `, - }, - { - name: 'package.json', - content: `{ - "dependencies": { - "foo": "^1.0.0", - "str-or-num": "^1.0.0" - } - }`, - }, - ]; - const cdn: CdnData = { - 'str-or-num': { - versions: { - '1.0.0': { - files: { - 'index.js': { - content: 'export const str = "str";', - }, - 'index.d.ts': { - content: 'declare export const str: string;', - }, - }, - }, - '2.0.0': { - files: { - 'index.js': { - content: 'export const num = 0;', - }, - 'index.d.ts': { - content: 'declare export const num: number;', - }, - }, - }, - }, - }, - foo: { - versions: { - '1.0.0': { - files: { - 'package.json': { - content: `{ - "dependencies": { - "str-or-num": "^2.0.0" - } - }`, - }, - 'index.js': { - content: `export * from 'str-or-num';`, - }, - 'index.d.ts': { - content: `declare export * from 'str-or-num';`, - }, - }, - }, - }, - }, - typescript: { - versions: { - '4.3.5': { - files: { - 'lib/lib.dom.d.ts': { - content: '', - }, - 'lib/lib.esnext.d.ts': { - content: '', - }, - 'lib/lib.dom.iterable.d.ts': { - content: '', - }, - }, - }, - }, - }, - }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { - name: 'index.js', - content: - 'import { num } from "./node_modules/foo@1.0.0/index.js";\nimport { str } from "./node_modules/str-or-num@1.0.0/index.js";\nconst a = str;\nconst b = num;\n', - contentType: 'text/javascript', - }, - }, - { - file: { - content: "export * from '../str-or-num@2.0.0/index.js';", - contentType: 'text/javascript; charset=utf-8', - name: 'node_modules/foo@1.0.0/index.js', - }, - kind: 'file', - }, - { - file: { - content: 'export const str = "str";', - contentType: 'text/javascript; charset=utf-8', - name: 'node_modules/str-or-num@1.0.0/index.js', - }, - kind: 'file', - }, - { - file: { - content: 'export const num = 0;', - contentType: 'text/javascript; charset=utf-8', - name: 'node_modules/str-or-num@2.0.0/index.js', - }, - kind: 'file', - }, - { - file: { - content: - '{\n "dependencies": {\n "foo": "^1.0.0",\n "str-or-num": "^1.0.0"\n }\n }', - name: 'package.json', - }, - kind: 'file', - }, - ]; - await checkTransform(files, expected, {}, cdn); - }); + // test('respects package.json dependency for semantic errors', async () => { + // const files: File[] = [ + // { + // name: 'index.ts', + // content: ` + // import {foo} from "foo"; + // foo(123); + // `, + // }, + // { + // name: 'package.json', + // content: `{ + // "dependencies": { + // "foo": "^1.0.0" + // } + // }`, + // }, + // ]; + // const cdn: CdnData = { + // foo: { + // // foo takes a string in 1.0.0, and a number in 2.0.0. We should expect + // // an error because we depend on 1.0.0. + // versions: { + // '1.0.0': { + // files: { + // 'index.js': { + // content: 'export const foo = (s) => s;', + // }, + // 'index.d.ts': { + // content: ` + // import type {t} from './type.js'; + // export declare const foo: (s: t) => t; + // `, + // }, + // 'type.d.ts': { + // content: `export type t = string;`, + // }, + // }, + // }, + // '2.0.0': { + // files: { + // 'index.js': { + // content: 'export const foo = (n) => n;', + // }, + // 'index.d.ts': { + // content: 'export declare const foo: (n: number) => number;', + // }, + // }, + // }, + // }, + // }, + // }; + // const expected: BuildOutput[] = [ + // { + // diagnostic: { + // code: 2345, + // message: + // "Argument of type 'number' is not assignable to parameter of type 'string'.", + // range: { + // end: { + // character: 17, + // line: 2, + // }, + // start: { + // character: 14, + // line: 2, + // }, + // }, + // severity: 1, + // source: 'typescript', + // }, + // filename: 'index.ts', + // kind: 'diagnostic', + // }, + // { + // kind: 'file', + // file: { + // name: 'index.js', + // content: + // 'import { foo } from "./node_modules/foo@1.0.0/index.js";\nfoo(123);\n', + // contentType: 'text/javascript', + // }, + // }, + // { + // kind: 'file', + // file: { + // name: 'node_modules/foo@1.0.0/index.js', + // content: 'export const foo = (s) => s;', + // contentType: 'text/javascript; charset=utf-8', + // }, + // }, + // { + // kind: 'file', + // file: { + // name: 'package.json', + // content: `{ + // "dependencies": { + // "foo": "^1.0.0" + // } + // }`, + // }, + // }, + // ]; + // await checkTransform(files, expected, {}, cdn); + // }); - test('compiles jsx file to js', async () => { - const files: SampleFile[] = [ - { - name: 'index.jsx', - content: ` - import React from "react"; - export const foo = (greeting) =>
{greeting}
- `, - }, - { - name: 'package.json', - content: `{ - "dependencies": { - "react": "^18.1.0" - } - }`, - }, - ]; + // test('prefers typings from: typings, types, main, index.d.ts', async () => { + // const wrong = { + // content: 'export type StringType = number;', + // }; + // const files: File[] = [ + // { + // name: 'index.ts', + // content: ` + // import {StringType} from "a"; + // export const str: StringType = "foo"; + // `, + // }, + // ]; + // const cdn: CdnData = { + // a: { + // versions: { + // '1.0.0': { + // files: { + // 'package.json': { + // content: `{ + // "typings": "typings.d.ts", + // "types": "types.d.ts", + // "main": "main.js" + // }`, + // }, + // 'typings.d.ts': { + // content: 'export * from "b";', + // }, + // 'types.d.ts': wrong, + // 'main.d.ts': wrong, + // 'index.d.ts': wrong, + // }, + // }, + // }, + // }, + // b: { + // versions: { + // '1.0.0': { + // files: { + // 'package.json': { + // content: `{ + // "types": "types.d.ts", + // "main": "main.js" + // }`, + // }, + // 'typings.d.ts': wrong, + // 'types.d.ts': { + // content: 'export * from "c";', + // }, + // 'main.d.ts': wrong, + // 'index.d.ts': wrong, + // }, + // }, + // }, + // }, + // c: { + // versions: { + // '1.0.0': { + // files: { + // 'package.json': { + // content: `{ + // "main": "main.js" + // }`, + // }, + // 'typings.d.ts': wrong, + // 'types.d.ts': wrong, + // 'main.d.ts': { + // content: 'export * from "d";', + // }, + // 'index.d.ts': wrong, + // }, + // }, + // }, + // }, + // d: { + // versions: { + // '1.0.0': { + // files: { + // 'package.json': { + // content: `{}`, + // }, + // 'typings.d.ts': wrong, + // 'types.d.ts': wrong, + // 'main.d.ts': wrong, + // 'index.d.ts': { + // content: 'export type StringType = string;', + // }, + // }, + // }, + // }, + // }, + // }; + // const expected: BuildOutput[] = [ + // { + // kind: 'file', + // file: { + // name: 'index.js', + // content: `export const str = "foo";\n`, + // contentType: 'text/javascript', + // }, + // }, + // ]; + // await checkTransform(files, expected, {}, cdn); + // }); - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { - name: 'index.js', - content: - 'import React from "./node_modules/react@18.1.0/index.js";\nexport const foo = (greeting) => React.createElement("div", null, greeting);\n', - contentType: 'text/javascript', - }, - }, - { - kind: 'file', - file: { - content: 'export const createElement = (tag, props, children) => {};', - contentType: 'text/javascript; charset=utf-8', - name: 'node_modules/react@18.1.0/index.js', - }, - }, - { - kind: 'file', - file: { - content: - '{\n "dependencies": {\n "react": "^18.1.0"\n }\n }', - name: 'package.json', - }, - }, - ]; + // test('handles multiple versions', async () => { + // const files: File[] = [ + // { + // name: 'index.ts', + // content: ` + // import {num} from "foo"; + // import {str} from "str-or-num"; + // const a: string = str; + // const b: number = num + // `, + // }, + // { + // name: 'package.json', + // content: `{ + // "dependencies": { + // "foo": "^1.0.0", + // "str-or-num": "^1.0.0" + // } + // }`, + // }, + // ]; + // const cdn: CdnData = { + // 'str-or-num': { + // versions: { + // '1.0.0': { + // files: { + // 'index.js': { + // content: 'export const str = "str";', + // }, + // 'index.d.ts': { + // content: 'declare export const str: string;', + // }, + // }, + // }, + // '2.0.0': { + // files: { + // 'index.js': { + // content: 'export const num = 0;', + // }, + // 'index.d.ts': { + // content: 'declare export const num: number;', + // }, + // }, + // }, + // }, + // }, + // foo: { + // versions: { + // '1.0.0': { + // files: { + // 'package.json': { + // content: `{ + // "dependencies": { + // "str-or-num": "^2.0.0" + // } + // }`, + // }, + // 'index.js': { + // content: `export * from 'str-or-num';`, + // }, + // 'index.d.ts': { + // content: `declare export * from 'str-or-num';`, + // }, + // }, + // }, + // }, + // }, + // typescript: { + // versions: { + // '4.3.5': { + // files: { + // 'lib/lib.dom.d.ts': { + // content: '', + // }, + // 'lib/lib.esnext.d.ts': { + // content: '', + // }, + // 'lib/lib.dom.iterable.d.ts': { + // content: '', + // }, + // }, + // }, + // }, + // }, + // }; + // const expected: BuildOutput[] = [ + // { + // kind: 'file', + // file: { + // name: 'index.js', + // content: + // 'import { num } from "./node_modules/foo@1.0.0/index.js";\nimport { str } from "./node_modules/str-or-num@1.0.0/index.js";\nconst a = str;\nconst b = num;\n', + // contentType: 'text/javascript', + // }, + // }, + // { + // file: { + // content: "export * from '../str-or-num@2.0.0/index.js';", + // contentType: 'text/javascript; charset=utf-8', + // name: 'node_modules/foo@1.0.0/index.js', + // }, + // kind: 'file', + // }, + // { + // file: { + // content: 'export const str = "str";', + // contentType: 'text/javascript; charset=utf-8', + // name: 'node_modules/str-or-num@1.0.0/index.js', + // }, + // kind: 'file', + // }, + // { + // file: { + // content: 'export const num = 0;', + // contentType: 'text/javascript; charset=utf-8', + // name: 'node_modules/str-or-num@2.0.0/index.js', + // }, + // kind: 'file', + // }, + // { + // file: { + // content: + // '{\n "dependencies": {\n "foo": "^1.0.0",\n "str-or-num": "^1.0.0"\n }\n }', + // name: 'package.json', + // }, + // kind: 'file', + // }, + // ]; + // await checkTransform(files, expected, {}, cdn); + // }); - const cdn: CdnData = { - react: { - versions: { - '18.1.0': { - files: { - 'index.js': { - content: - 'export const createElement = (tag, props, children) => {};', - }, - }, - }, - }, - }, - }; + // test('compiles jsx file to js', async () => { + // const files: File[] = [ + // { + // name: 'index.jsx', + // content: ` + // import React from "react"; + // export const foo = (greeting) =>
{greeting}
+ // `, + // }, + // { + // name: 'package.json', + // content: `{ + // "dependencies": { + // "react": "^18.1.0" + // } + // }`, + // }, + // ]; - await checkTransform(files, expected, {}, cdn); - }); + // const expected: BuildOutput[] = [ + // { + // kind: 'file', + // file: { + // name: 'index.js', + // content: + // 'import React from "./node_modules/react@18.1.0/index.js";\nexport const foo = (greeting) => React.createElement("div", null, greeting);\n', + // contentType: 'text/javascript', + // }, + // }, + // { + // kind: 'file', + // file: { + // content: 'export const createElement = (tag, props, children) => {};', + // contentType: 'text/javascript; charset=utf-8', + // name: 'node_modules/react@18.1.0/index.js', + // }, + // }, + // { + // kind: 'file', + // file: { + // content: + // '{\n "dependencies": {\n "react": "^18.1.0"\n }\n }', + // name: 'package.json', + // }, + // }, + // ]; - test('compiles tsx file to js', async () => { - const files: SampleFile[] = [ - { - name: 'index.tsx', - content: ` - import * as React from "react"; - export const foo = (greeting: string) =>
{greeting}
- `, - }, - { - name: 'package.json', - content: `{ - "dependencies": { - "react": "^18.1.0" - } - }`, - }, - ]; + // const cdn: CdnData = { + // react: { + // versions: { + // '18.1.0': { + // files: { + // 'index.js': { + // content: + // 'export const createElement = (tag, props, children) => {};', + // }, + // }, + // }, + // }, + // }, + // }; - const expected: BuildOutput[] = [ - { - kind: 'file', - file: { - name: 'index.js', - content: - 'import * as React from "./node_modules/react@18.1.0/index.js";\nexport const foo = (greeting) => React.createElement("div", null, greeting);\n', - contentType: 'text/javascript', - }, - }, - { - kind: 'file', - file: { - content: 'export const createElement = (tag, props, children) => {};', - contentType: 'text/javascript; charset=utf-8', - name: 'node_modules/react@18.1.0/index.js', - }, - }, - { - kind: 'file', - file: { - content: - '{\n "dependencies": {\n "react": "^18.1.0"\n }\n }', - name: 'package.json', - }, - }, - ]; + // await checkTransform(files, expected, {}, cdn); + // }); - const cdn: CdnData = { - react: { - versions: { - '18.1.0': { - files: { - 'index.js': { - content: - 'export const createElement = (tag, props, children) => {};', - }, - 'index.d.ts': { - content: - 'declare export const createElement(tag: unknown, props: unknown, children: unknown) => unknown;', - }, - }, - }, - }, - }, - }; + // test('compiles tsx file to js', async () => { + // const files: File[] = [ + // { + // name: 'index.tsx', + // content: ` + // import * as React from "react"; + // export const foo = (greeting: string) =>
{greeting}
+ // `, + // }, + // { + // name: 'package.json', + // content: `{ + // "dependencies": { + // "react": "^18.1.0" + // } + // }`, + // }, + // ]; - await checkTransform(files, expected, {}, cdn); - }); + // const expected: BuildOutput[] = [ + // { + // kind: 'file', + // file: { + // name: 'index.js', + // content: + // 'import * as React from "./node_modules/react@18.1.0/index.js";\nexport const foo = (greeting) => React.createElement("div", null, greeting);\n', + // contentType: 'text/javascript', + // }, + // }, + // { + // kind: 'file', + // file: { + // content: 'export const createElement = (tag, props, children) => {};', + // contentType: 'text/javascript; charset=utf-8', + // name: 'node_modules/react@18.1.0/index.js', + // }, + // }, + // { + // kind: 'file', + // file: { + // content: + // '{\n "dependencies": {\n "react": "^18.1.0"\n }\n }', + // name: 'package.json', + // }, + // }, + // ]; + + // const cdn: CdnData = { + // react: { + // versions: { + // '18.1.0': { + // files: { + // 'index.js': { + // content: + // 'export const createElement = (tag, props, children) => {};', + // }, + // 'index.d.ts': { + // content: + // 'declare export const createElement(tag: unknown, props: unknown, children: unknown) => unknown;', + // }, + // }, + // }, + // }, + // }, + // }; + + // await checkTransform(files, expected, {}, cdn); + // }); }); diff --git a/src/test/worker-test-util.ts b/src/test/worker-test-util.ts index 79c7ed7d..8e51b0c0 100644 --- a/src/test/worker-test-util.ts +++ b/src/test/worker-test-util.ts @@ -8,12 +8,13 @@ import {assert} from '@esm-bundle/chai'; import {build} from '../typescript-worker/build.js'; import {executeServerCommand} from '@web/test-runner-commands'; -import { - BuildOutput, +import type { + BuildResult, ModuleImportMap, - SampleFile, + File, + FileDiagnostic, } from '../shared/worker-api.js'; -import {CdnData} from './fake-cdn-plugin.js'; +import type {CdnData} from './fake-cdn-plugin.js'; export const configureFakeCdn = async ( data: CdnData @@ -32,55 +33,57 @@ export const configureFakeCdn = async ( }; export const checkTransform = async ( - files: SampleFile[], - expected: BuildOutput[], + files: File[], + expected: BuildResult, importMap: ModuleImportMap = {}, cdnData: CdnData = {} ) => { const {cdnBaseUrl, deleteCdnData} = await configureFakeCdn(cdnData); try { - const results: BuildOutput[] = []; - await new Promise((resolve) => { - const emit = (result: BuildOutput) => { - if (result.kind === 'done') { - resolve(); - } else { - results.push(result); - } - }; - build(files, {importMap, cdnBaseUrl}, emit); - }); + const result = await build(files, {importMap, cdnBaseUrl}); - for (const result of results) { - if (result.kind === 'diagnostic') { - // Sometimes diagnostics contain a CDN URL to help with debugging - // (usually the unpkg.com URL). But that will be a local dynamic URL in - // testing, so we'll substitute a static string so that we can do a - // simple equality test. - while (result.diagnostic.message.includes(cdnBaseUrl)) { - result.diagnostic.message = result.diagnostic.message.replace( - cdnBaseUrl, - '/' - ); - } + // console.log('build result', result); + + for (const {diagnostic} of result.diagnostics) { + // Sometimes diagnostics contain a CDN URL to help with debugging + // (usually the unpkg.com URL). But that will be a local dynamic URL in + // testing, so we'll substitute a static string so that we can do a + // simple equality test. + while (diagnostic.message.includes(cdnBaseUrl)) { + diagnostic.message = diagnostic.message.replace( + cdnBaseUrl, + '/' + ); } } assert.deepEqual( - results.sort(sortBuildOutput), - expected.sort(sortBuildOutput) + result.files.sort(compareFiles), + expected.files.sort(compareFiles) + ); + assert.deepEqual( + result.diagnostics.sort(compareDiagnostics), + expected.diagnostics.sort(compareDiagnostics) ); + if (expected.semanticDiagnostics !== undefined) { + assert.isDefined(result.semanticDiagnostics); + assert.deepEqual( + (await result.semanticDiagnostics!).sort(compareDiagnostics), + (await expected.semanticDiagnostics).sort(compareDiagnostics) + ); + } } finally { await deleteCdnData(); } }; -const sortBuildOutput = (a: BuildOutput, b: BuildOutput) => { - if (a.kind === 'file' && b.kind === 'file') { - return a.file.name.localeCompare(b.file.name); - } - if (a.kind === 'diagnostic' && b.kind === 'diagnostic') { - return a.diagnostic.message.localeCompare(b.diagnostic.message); - } - return a.kind.localeCompare(b.kind); +const compareFiles = (a: File, b: File) => { + return a.name.localeCompare(b.name); +}; + +const compareDiagnostics = (a: FileDiagnostic, b: FileDiagnostic) => { + return ( + a.filename.localeCompare(b.filename) || + a.diagnostic.message.localeCompare(b.diagnostic.message) + ); }; diff --git a/src/typescript-worker/bare-module-transformer.ts b/src/typescript-worker/bare-module-transformer.ts index 2efe4195..3fddaa9a 100644 --- a/src/typescript-worker/bare-module-transformer.ts +++ b/src/typescript-worker/bare-module-transformer.ts @@ -6,26 +6,21 @@ import * as esModuleLexer from 'es-module-lexer'; import { - MergedAsyncIterables, parseNpmStyleSpecifier, resolveUrlPath, charToLineAndChar, fileExtension, classifySpecifier, relativeUrlPath, + type NpmFileLocation, + npmFileLocationToString, } from './util.js'; -import {Deferred} from '../shared/deferred.js'; import {NodeModuleResolver} from './node-module-resolver.js'; -import { - BuildOutput, - DiagnosticBuildOutput, - FileBuildOutput, - SampleFile, -} from '../shared/worker-api.js'; +import type {File, Diagnostic} from '../shared/worker-api.js'; import {ImportMapResolver} from './import-map-resolver.js'; import {CachingCdn} from './caching-cdn.js'; -import {NpmFileLocation, PackageJson} from './util.js'; +import type {PackageJson} from './util.js'; /** * Transforms bare module specifiers in .js files to canonical local paths, and @@ -52,7 +47,7 @@ import {NpmFileLocation, PackageJson} from './util.js'; export class BareModuleTransformer { private _cdn: CachingCdn; private _importMapResolver: ImportMapResolver; - private _emittedExternalDependencies = new Set(); + // private _emittedExternalDependencies = new Set(); private _nodeResolver = new NodeModuleResolver({ conditions: ['module', 'import', 'development', 'browser'], }); @@ -62,78 +57,89 @@ export class BareModuleTransformer { this._importMapResolver = importMapResolver; } - async *process( - results: AsyncIterable | Iterable - ): AsyncIterable { - // This "output" iterable helps us emit all build outputs as soon as they - // are available as we asynchronously walk the dependency tree. - const output = new MergedAsyncIterables(); - output.add(this._handleProjectFiles(results, output)); - yield* output; - } + // async *process( + // results: AsyncIterable | Iterable + // ): AsyncIterable { + // // This "output" iterable helps us emit all build outputs as soon as they + // // are available as we asynchronously walk the dependency tree. + // const output = new MergedAsyncIterables(); + // output.add(this._handleProjectFiles(results, output)); + // yield* output; + // } - /** - * Handle files from the top-level project. - */ - private async *_handleProjectFiles( - results: AsyncIterable | Iterable, - output: MergedAsyncIterables - ): AsyncIterable { - // The project might contain a package.json, which will determine our - // top-level version constraints. Resolve it lazily. - const packageJson = new Deferred(); - const getPackageJson = () => packageJson.promise; - for await (const result of results) { - if (result.kind === 'file' && result.file.name.endsWith('.js')) { - output.add(this._handleModule(result, getPackageJson, output)); - } else { - yield result; - if (result.kind === 'file' && result.file.name === 'package.json') { - let parsed: PackageJson | undefined; - try { - parsed = JSON.parse(result.file.content) as PackageJson; - } catch (e) { - yield makeJsonParseDiagnostic(e as Error, result.file); - } - if (parsed !== undefined) { - packageJson.resolve(parsed); - } - } - } - } - if (!packageJson.settled) { - // We never found a package.json. - packageJson.resolve(undefined); - } - } + // /** + // * Handle files from the top-level project. + // */ + // private async *_handleProjectFiles( + // results: AsyncIterable | Iterable, + // output: MergedAsyncIterables + // ): AsyncIterable { + // // The project might contain a package.json, which will determine our + // // top-level version constraints. Resolve it lazily. + // const packageJson = new Deferred(); + // const getPackageJson = () => packageJson.promise; + // for await (const result of results) { + // if (result.kind === 'file' && result.file.name.endsWith('.js')) { + // output.add(this._handleModule(result, getPackageJson, output)); + // } else { + // yield result; + // if (result.kind === 'file' && result.file.name === 'package.json') { + // let parsed: PackageJson | undefined; + // try { + // parsed = JSON.parse(result.file.content) as PackageJson; + // } catch (e) { + // yield makeJsonParseDiagnostic(e as Error, result.file); + // } + // if (parsed !== undefined) { + // packageJson.resolve(parsed); + // } + // } + // } + // } + // if (!packageJson.settled) { + // // We never found a package.json. + // packageJson.resolve(undefined); + // } + // } /** * Transform all of the imported module specifiers in the given JS module, * emit the transformed file, and process any dependencies corresponding to * those specifiers. */ - private async *_handleModule( - file: FileBuildOutput, - getPackageJson: () => Promise, - output: MergedAsyncIterables - ): AsyncIterable { - let js = file.file.content; + async transformModule( + file: File, + packageJson?: PackageJson | undefined + ): Promise<{ + file?: File; + diagnostics: Array; + /** A set of versioned npm location strings */ + externalLocations?: Set; + }> { + const diagnostics: Array = []; + + let js = file.content; let specifiers; + await esModuleLexer.init; + try { [specifiers] = esModuleLexer.parse(js); } catch (e) { - yield file; - const diagnostic = makeEsModuleLexerDiagnostic( - e as Error, - file.file.name - ); + const diagnostic = makeEsModuleLexerDiagnostic(e as Error); if (diagnostic !== undefined) { - yield diagnostic; + diagnostics.push(diagnostic); } - return; + return {file, diagnostics}; } - const transforms = []; + const transforms: Array<{ + info: esModuleLexer.ImportSpecifier; + newSpecifierPromise: Promise<{ + resolved: string; + location?: NpmFileLocation | undefined; + }>; + }> = []; + const externalLocations = new Set(); // Note we iterating backwards so that the character offsets are not // invalidated after each substitution. for (let i = specifiers.length - 1; i >= 0; i--) { @@ -147,9 +153,8 @@ export class BareModuleTransformer { info: specifiers[i], newSpecifierPromise: this._handleSpecifier( oldSpecifier, - file.file.name, - getPackageJson, - output + file.name, + packageJson ), }); } @@ -157,38 +162,60 @@ export class BareModuleTransformer { info: {s: start, e: end, n: oldSpecifier, d: dynamicStart}, newSpecifierPromise, } of transforms) { - let newSpecifier; + let newSpecifier: { + resolved: string; + location?: NpmFileLocation; + }; try { newSpecifier = await newSpecifierPromise; + // console.log('newSpecifier', newSpecifier); } catch (e) { // TODO(aomarks) If this was a TypeScript file, the user isn't going to // see this diagnostic, since we're looking at the JS file. To show it // correctly on the original file, we'll need source maps support. - yield { - kind: 'diagnostic', - filename: file.file.name, - diagnostic: { - message: `Could not resolve module "${oldSpecifier}": ${ - (e as Error).message - }`, - range: { - start: charToLineAndChar(js, start), - end: charToLineAndChar(js, end), - }, + diagnostics.push({ + message: `Could not resolve module "${oldSpecifier}": ${ + (e as Error).message + }`, + range: { + start: charToLineAndChar(js, start), + end: charToLineAndChar(js, end), }, - }; + }); + // yield { + // kind: 'diagnostic', + // filename: file.file.name, + // diagnostic: { + // message: `Could not resolve module "${oldSpecifier}": ${ + // (e as Error).message + // }`, + // range: { + // start: charToLineAndChar(js, start), + // end: charToLineAndChar(js, end), + // }, + // }, + // }; continue; } - if (newSpecifier === oldSpecifier) { + if (newSpecifier.location !== undefined) { + externalLocations.add(npmFileLocationToString(newSpecifier.location)); + } + if (newSpecifier.resolved === oldSpecifier) { continue; } // For dynamic imports, the start/end range doesn't include quotes. const isDynamic = dynamicStart !== -1; - const replacement = isDynamic ? `'${newSpecifier}'` : newSpecifier; + const replacement = isDynamic + ? `'${newSpecifier.resolved}'` + : newSpecifier.resolved; js = js.substring(0, start) + replacement + js.substring(end); } - file.file.content = js; - yield file; + file.content = js; + return { + file, + diagnostics, + externalLocations, + }; } /** @@ -198,36 +225,32 @@ export class BareModuleTransformer { private async _handleSpecifier( specifier: string, referrer: string, - getPackageJson: () => Promise, - output: MergedAsyncIterables - ): Promise { + packageJson?: PackageJson + ): Promise<{resolved: string; location?: NpmFileLocation}> { const fromImportMap = this._importMapResolver.resolve(specifier); if (fromImportMap !== null) { - return fromImportMap; + return {resolved: fromImportMap}; } const kind = classifySpecifier(specifier); + // console.log('_handleSpecifier kind', specifier, kind, referrer); if (kind === 'url') { - return specifier; + return {resolved: specifier}; } if (kind === 'bare') { - return this._handleBareSpecifier( - specifier, - referrer, - getPackageJson, - output - ); + return this._handleBareSpecifier(specifier, referrer, packageJson); } // Anything else is a relative specifier. if (!referrer.startsWith('node_modules/')) { // A relative specifier within a top-level project file. This specifier // must resolve to another top-level project file, so there's nothing more // we need to do here. - return specifier; + return {resolved: specifier}; } // E.g. if `referrer` is "node_modules/foo@1.2.3/a.js" and `specifier` is // "./b.js", then `absolute` will be "node_modules/foo@1.2.3/b.js", and // `bare` will be "foo@1.2.3/b.js". const absolute = resolveUrlPath(referrer, specifier); + // console.log('_handleSpecifier absolute', absolute); const bare = absolute.slice('/node_modules/'.length); if (!fileExtension(specifier)) { // We can't simply return the existing relative specifier if there's no @@ -242,8 +265,7 @@ export class BareModuleTransformer { // already included in the specifier itself at this point. We also // wouldn't want to pass this scope's `getPackageJson`, because it would // be the wrong one. - async () => undefined, - output + undefined ); } // This relative specifier is good as-is, since it has an extension. We just @@ -252,27 +274,26 @@ export class BareModuleTransformer { if (parsed === undefined) { throw new Error(`Invalid specifier "${bare}"`); } - output.add(this._fetchExternalDependency(parsed, output)); - return specifier; + // output.add(this._fetchExternalDependency(parsed, output)); + return {resolved: specifier, location: parsed}; } /** - * Canonicalize the given bare module specifier, then fetch it and add it to - * the local filesystem. + * Canonicalize the given bare module specifier into a relative path from the + * referrer, and return the specifier's NPM location for further processing. */ private async _handleBareSpecifier( specifier: string, referrer: string, - getPackageJson: () => Promise, - output: MergedAsyncIterables - ): Promise { + rootPackageJson?: PackageJson + ): Promise<{resolved: string; location: NpmFileLocation}> { let location = parseNpmStyleSpecifier(specifier); if (location === undefined) { throw new Error(`Invalid specifier "${specifier}"`); } if (!location.version) { location.version = - (await getPackageJson())?.dependencies?.[location.pkg] ?? 'latest'; + rootPackageJson?.dependencies?.[location.pkg] ?? 'latest'; } // Resolve the version number before resolving the module, so that any error // messages generated by the NodeModuleResolver will contain a concrete @@ -290,73 +311,71 @@ export class BareModuleTransformer { // step directly into the NodeResolver class. location = await this._cdn.canonicalize(location); } - output.add(this._fetchExternalDependency(location, output)); + // already handled by return: + // output.add(this._fetchExternalDependency(location, output)); const absolute = `node_modules/${location.pkg}@${location.version}/${location.path}`; const relative = relativeUrlPath(referrer, absolute); - return relative; + return {resolved: relative, location}; } - /** - * Fetch the given external module, and add it to the local filesystem under - * its "node_modules/" path. - */ - private async *_fetchExternalDependency( - location: NpmFileLocation, - output: MergedAsyncIterables - ) { - const path = `${location.pkg}@${location.version}/${location.path}`; - if (this._emittedExternalDependencies.has(path)) { - // We already emitted this dependency. Avoid import loops and wasteful - // double fetches. - return; - } - this._emittedExternalDependencies.add(path); - let asset; - try { - asset = await this._cdn.fetch(location); - } catch (e) { - // TODO(aomarks) This file will end up as a 404 error when fetched from - // the preview iframe, because we're simply omitting this file from our - // output on error. We should instead allow FileBuildOutput to carry an - // HTTP status code, so then we could propagate this specific error to be - // served by the service worker, so that it shows up more usefully in the - // network tab. - console.error(`Error fetching ${path} from CDN: ${(e as Error).message}`); - return; - } - let packageJson: PackageJson | undefined | null = null; - const getPackageJson = async (): Promise => { - if (packageJson === null) { - try { - packageJson = await this._cdn.fetchPackageJson(location); - } catch { - packageJson = undefined; - } - } - return packageJson; - }; - yield* this._handleModule( - { - kind: 'file', - file: { - name: `node_modules/${path}`, - content: asset.content, - contentType: asset.contentType, - }, - }, - getPackageJson, - output - ); - } + // /** + // * Fetch the given external module, and add it to the local filesystem under + // * its "node_modules/" path. + // */ + // private async *_fetchExternalDependency( + // location: NpmFileLocation, + // output: MergedAsyncIterables + // ) { + // const path = `${location.pkg}@${location.version}/${location.path}`; + // if (this._emittedExternalDependencies.has(path)) { + // // We already emitted this dependency. Avoid import loops and wasteful + // // double fetches. + // return; + // } + // this._emittedExternalDependencies.add(path); + // let asset; + // try { + // asset = await this._cdn.fetch(location); + // } catch (e) { + // // TODO(aomarks) This file will end up as a 404 error when fetched from + // // the preview iframe, because we're simply omitting this file from our + // // output on error. We should instead allow FileBuildOutput to carry an + // // HTTP status code, so then we could propagate this specific error to be + // // served by the service worker, so that it shows up more usefully in the + // // network tab. + // console.error(`Error fetching ${path} from CDN: ${(e as Error).message}`); + // return; + // } + // let packageJson: PackageJson | undefined | null = null; + // const getPackageJson = async (): Promise => { + // if (packageJson === null) { + // try { + // packageJson = await this._cdn.fetchPackageJson(location); + // } catch { + // packageJson = undefined; + // } + // } + // return packageJson; + // }; + // yield* this._handleModule( + // { + // kind: 'file', + // file: { + // name: `node_modules/${path}`, + // content: asset.content, + // contentType: asset.contentType, + // }, + // }, + // getPackageJson, + // output + // ); + // } } /** * Create a useful Playground diagnostic from an es-module-lexer exception. */ -const makeEsModuleLexerDiagnostic = ( - e: Error, - filename: string -): DiagnosticBuildOutput | undefined => { +const makeEsModuleLexerDiagnostic = (e: Error): Diagnostic | undefined => { const match = e.message.match(/@:(\d+):(\d+)$/); if (match === null) { return undefined; @@ -364,63 +383,10 @@ const makeEsModuleLexerDiagnostic = ( const line = Number(match[1]) - 1; const character = Number(match[2]) - 1; return { - kind: 'diagnostic', - filename, - diagnostic: { - message: `es-module-lexer error: ${e.message}`, - range: { - start: {line, character}, - end: {line, character: character + 1}, - }, - }, - }; -}; - -/** - * Create a useful Playground diagnostic from a JSON.parse exception. - */ -const makeJsonParseDiagnostic = ( - e: Error, - file: SampleFile -): DiagnosticBuildOutput => { - const start = extractPositionFromJsonParseError(e.message, file.content) ?? { - line: 0, - character: 0, - }; - return { - kind: 'diagnostic', - filename: file.name, - diagnostic: { - message: `Invalid package.json: ${e}`, - range: { - start, - // To the rest of the file. - end: charToLineAndChar(file.content, file.content.length), - }, + message: `es-module-lexer error: ${e.message}`, + range: { + start: {line, character}, + end: {line, character: character + 1}, }, }; }; - -/** - * Try to extract the line and character from a `JSON.parse` exception message. - * - * Chrome and Firefox often include this information, but using different - * formats. Safari never includes this information. - */ -const extractPositionFromJsonParseError = ( - message: string, - json: string -): {line: number; character: number} | undefined => { - const chrome = message.match(/at position (\d+)/); - if (chrome !== null) { - return charToLineAndChar(json, Number(chrome[1])); - } - const firefox = message.match(/at line (\d+) column (\d+)/); - if (firefox !== null) { - return { - line: Number(firefox[1]) - 1, - character: Number(firefox[2]) - 1, - }; - } - return undefined; -}; diff --git a/src/typescript-worker/build.ts b/src/typescript-worker/build.ts index 00fd1191..26e172ff 100644 --- a/src/typescript-worker/build.ts +++ b/src/typescript-worker/build.ts @@ -6,29 +6,177 @@ import {BareModuleTransformer} from './bare-module-transformer.js'; -import {SampleFile, BuildOutput, WorkerConfig} from '../shared/worker-api.js'; +import type { + File, + BuildResult, + WorkerConfig, + Diagnostic, + FileDiagnostic, +} from '../shared/worker-api.js'; import {getWorkerContext} from './worker-context.js'; -import {processTypeScriptFiles} from './typescript-builder.js'; +import {buildTypeScript} from './typescript-builder.js'; +import { + type PackageJson, + charToLineAndChar, + parseNpmStyleSpecifier, +} from './util.js'; export const build = async ( - files: Array, - config: WorkerConfig, - emit: (result: BuildOutput) => void -): Promise => { + files: Array, + config: WorkerConfig +): Promise => { const workerContext = getWorkerContext(config); - const bareModuleBuilder = new BareModuleTransformer( + const bareModuleTransformer = new BareModuleTransformer( workerContext.cdn, workerContext.importMapResolver ); - const results = bareModuleBuilder.process( - processTypeScriptFiles( - workerContext, - files.map((file) => ({kind: 'file', file})) - ) + const {data: projectPackageJson, diagnostic: packageJsonDiagnostic} = + getPackageJson(files); + + const buildResult = await buildTypeScript( + workerContext, + files, + projectPackageJson ); - for await (const result of results) { - emit(result); + + if (packageJsonDiagnostic !== undefined) { + buildResult.diagnostics.push(packageJsonDiagnostic); + } + + // Trasnform bare module specifiers in all files, and load external + // dependencies. + // TODO (justinfagnani): Maybe stop loading external dependencies statically. + // Instead let the service worker handle fetches for them, then transform + // them. Or maybe just stop handling dynamic imports. + + // Files to transform to resolve bare module specifiers. + // Entries that are Files are project file and contain their own source test. + // Entries that are strings are external files and need to be fetched from the + // CDN. + const filesToResolve = new Set(); + buildResult.files.forEach((file) => filesToResolve.add(file)); + + for (const fileOrLocation of filesToResolve) { + let file: File; + let packageJson: PackageJson | undefined; + if (typeof fileOrLocation === 'string') { + // External files are represented by strings. + const location = parseNpmStyleSpecifier(fileOrLocation); + if (location === undefined) { + continue; + } + try { + const cdnFile = await workerContext.cdn.fetch(location); + file = { + name: `node_modules/${fileOrLocation}`, + ...cdnFile, + }; + buildResult.files.push(file); + } catch (e) { + // TODO(aomarks) This file will end up as a 404 error when fetched from + // the preview iframe, because we're simply omitting this file from our + // output on error. We should instead allow FileBuildOutput to carry an + // HTTP status code, so then we could propagate this specific error to be + // served by the service worker, so that it shows up more usefully in the + // network tab. + console.error( + `Error fetching ${fileOrLocation} from CDN: ${(e as Error).message}` + ); + continue; + } + try { + packageJson = await workerContext.cdn.fetchPackageJson(location); + } catch { + packageJson = undefined; + } + } else { + // Only project files are represented by File objects. + file = fileOrLocation; + packageJson = projectPackageJson; + } + const result = await bareModuleTransformer.transformModule( + file, + packageJson + ); + + if (result.file !== undefined) { + file.content = result.file.content; + } + buildResult.diagnostics.push( + ...result.diagnostics.map((d) => ({ + filename: file.name, + diagnostic: d, + })) + ); + result.externalLocations?.forEach((location) => + filesToResolve.add(location) + ); + } + + return buildResult; +}; + +/** + * Create a useful Playground diagnostic from a JSON.parse exception. + */ +const makeJsonParseDiagnostic = (e: Error, file: File): Diagnostic => { + const start = extractPositionFromJsonParseError(e.message, file.content) ?? { + line: 0, + character: 0, + }; + return { + message: `Invalid package.json: ${e}`, + range: { + start, + // To the rest of the file. + end: charToLineAndChar(file.content, file.content.length), + }, + }; +}; + +/** + * Try to extract the line and character from a `JSON.parse` exception message. + * + * Chrome and Firefox often include this information, but using different + * formats. Safari never includes this information. + */ +const extractPositionFromJsonParseError = ( + message: string, + json: string +): {line: number; character: number} | undefined => { + const chrome = message.match(/at position (\d+)/); + if (chrome !== null) { + return charToLineAndChar(json, Number(chrome[1])); + } + const firefox = message.match(/at line (\d+) column (\d+)/); + if (firefox !== null) { + return { + line: Number(firefox[1]) - 1, + character: Number(firefox[2]) - 1, + }; + } + return undefined; +}; + +const getPackageJson = ( + files: Array +): {data: PackageJson | undefined; diagnostic: FileDiagnostic | undefined} => { + const packageJsonFile = files.find((file) => file.name === 'package.json'); + let projectPackageJson: PackageJson | undefined; + let packageJsonDiagnostic: FileDiagnostic | undefined; + if (packageJsonFile !== undefined) { + try { + projectPackageJson = JSON.parse(packageJsonFile.content); + } catch (e) { + packageJsonDiagnostic = { + filename: packageJsonFile!.name, + diagnostic: makeJsonParseDiagnostic(e as Error, packageJsonFile), + }; + } } - emit({kind: 'done'}); + return { + data: projectPackageJson, + diagnostic: packageJsonDiagnostic, + }; }; diff --git a/src/typescript-worker/playground-typescript-worker.ts b/src/typescript-worker/playground-typescript-worker.ts index 4d4520ae..7c64be47 100755 --- a/src/typescript-worker/playground-typescript-worker.ts +++ b/src/typescript-worker/playground-typescript-worker.ts @@ -6,11 +6,37 @@ import {expose} from 'comlink'; import {build} from './build.js'; -import {WorkerAPI} from '../shared/worker-api.js'; +import { + File, + BuildResult, + WorkerAPI, + WorkerConfig, + FileDiagnostic, +} from '../shared/worker-api.js'; import {getCompletionItemDetails, queryCompletions} from './completions.js'; +// Note: Comlink can only proxy objects/function that are direct parameters or +// return values of exposed functions. So we can't return an object with a +// Promise of semantic diagnostics or a function to retrieve them. Instead, we +// must take a callback function. There may be a way to write a Comlink +// custom handler to allow passing an object with proxies in it. const workerAPI: WorkerAPI = { - compileProject: build, + compileProject: async ( + files: Array, + config: WorkerConfig, + onSemanticDiagnostics?: (diagnostics?: Array) => void + ): Promise => { + const result = await build(files, config); + if (onSemanticDiagnostics !== undefined) { + void Promise.resolve(result.semanticDiagnostics).then( + onSemanticDiagnostics + ); + } + return { + files: result.files, + diagnostics: result.diagnostics, + }; + }, getCompletions: queryCompletions, getCompletionItemDetails: getCompletionItemDetails, }; diff --git a/src/typescript-worker/typescript-builder.ts b/src/typescript-worker/typescript-builder.ts index 8041fb04..5a5c544b 100644 --- a/src/typescript-worker/typescript-builder.ts +++ b/src/typescript-worker/typescript-builder.ts @@ -4,44 +4,34 @@ * SPDX-License-Identifier: BSD-3-Clause */ -import {BuildOutput, SampleFile} from '../shared/worker-api.js'; +import {BuildResult, File, FileDiagnostic} from '../shared/worker-api.js'; import {TypesFetcher} from './types-fetcher.js'; import {PackageJson} from './util.js'; import {makeLspDiagnostic} from './diagnostic.js'; import {WorkerContext} from './worker-context.js'; -export async function* processTypeScriptFiles( +const isTypeScriptInput = (file: File) => + file.name.endsWith('.ts') || + file.name.endsWith('.jsx') || + file.name.endsWith('.tsx'); + +export const buildTypeScript = async ( workerContext: WorkerContext, - results: AsyncIterable | Iterable -): AsyncIterable { + files: Array, + packageJson?: PackageJson | undefined +): Promise => { // Instantiate langservice variables for ease of access const langService = workerContext.languageServiceContext.service; const langServiceHost = workerContext.languageServiceContext.serviceHost; - let packageJson: PackageJson | undefined; - const compilerInputs = []; - for await (const result of results) { - if ( - result.kind === 'file' && - (result.file.name.endsWith('.ts') || - result.file.name.endsWith('.jsx') || - result.file.name.endsWith('.tsx')) - ) { - compilerInputs.push(result.file); - } else { - yield result; - if (result.kind === 'file' && result.file.name === 'package.json') { - try { - packageJson = JSON.parse(result.file.content) as PackageJson; - } catch (e) { - // A bit hacky, but BareModuleTransformer already emits a diagnostic - // for this case, so we don't need another one. - } - } - } - } + + const diagnostics: Array = []; + + // Initialize output files with all non-TypeScript files: + const outputFiles: Array = files.filter((f) => !isTypeScriptInput(f)); + const compilerInputs = files.filter(isTypeScriptInput); if (compilerInputs.length === 0) { - return; + return {files, diagnostics}; } // Immediately resolve local project files, and begin fetching types (but @@ -86,14 +76,13 @@ export async function* processTypeScriptFiles( for (const {file, url} of inputFiles) { for (const tsDiagnostic of langService.getSyntacticDiagnostics(url)) { - yield { - kind: 'diagnostic', + diagnostics.push({ filename: file.name, diagnostic: makeLspDiagnostic(tsDiagnostic), - }; + }); } const sourceFile = program.getSourceFile(url); - let compiled: SampleFile | undefined = undefined; + let compiled: File | undefined = undefined; program!.emit(sourceFile, (url, content) => { compiled = { name: new URL(url).pathname.slice(1), @@ -102,32 +91,38 @@ export async function* processTypeScriptFiles( }; }); if (compiled !== undefined) { - yield {kind: 'file', file: compiled}; + outputFiles.push(compiled); } } + const semanticDiagnostics: Promise> = (async () => { + // Wait for all typings to be fetched, and then retrieve slower semantic + // diagnostics. + const typings = await TypesFetcher.fetchTypes( + workerContext.cdn, + workerContext.importMapResolver, + packageJson, + inputFiles.map((file) => file.file.content), + workerContext.languageServiceContext.compilerOptions.lib + ); - // Wait for all typings to be fetched, and then retrieve slower semantic - // diagnostics. - const typings = await TypesFetcher.fetchTypes( - workerContext.cdn, - workerContext.importMapResolver, - packageJson, - inputFiles.map((file) => file.file.content), - workerContext.languageServiceContext.compilerOptions.lib - ); - for (const [path, content] of typings.files) { - // TypeScript is going to look for these files as paths relative to our - // source files, so we need to add them to our filesystem with those URLs. - const url = new URL(`node_modules/${path}`, self.origin).href; - langServiceHost.updateFileContentIfNeeded(url, content); - } - for (const {file, url} of inputFiles) { - for (const tsDiagnostic of langService.getSemanticDiagnostics(url)) { - yield { - kind: 'diagnostic', - filename: file.name, - diagnostic: makeLspDiagnostic(tsDiagnostic), - }; + for (const [path, content] of typings.files) { + // TypeScript is going to look for these files as paths relative to our + // source files, so we need to add them to our filesystem with those URLs. + const url = new URL(`node_modules/${path}`, self.origin).href; + langServiceHost.updateFileContentIfNeeded(url, content); } - } -} + + const diagnostics: Array = []; + for (const {file, url} of inputFiles) { + for (const tsDiagnostic of langService.getSemanticDiagnostics(url)) { + diagnostics.push({ + filename: file.name, + diagnostic: makeLspDiagnostic(tsDiagnostic), + }); + } + } + return diagnostics; + })(); + + return {files: outputFiles, diagnostics, semanticDiagnostics}; +}; diff --git a/src/typescript-worker/util.ts b/src/typescript-worker/util.ts index 58242f2d..703a3c50 100644 --- a/src/typescript-worker/util.ts +++ b/src/typescript-worker/util.ts @@ -138,6 +138,12 @@ export const parseNpmStyleSpecifier = ( return {pkg, version: version ?? '', path}; }; +export const npmFileLocationToString = ({ + pkg, + version, + path, +}: NpmFileLocation) => `${pkg}${version === '' ? '' : `@${version}`}/${path}`; + /** * Return the file extension of the given URL path. Does not include the leading * ".". Note this only considers the final ".", so e.g. given "foo.d.ts" this