From e8032ae84aebcb570411521296b6615b11b35606 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Mon, 13 Nov 2023 22:57:01 +0000 Subject: [PATCH 01/35] Flesh out the sync Compiler class --- lib/src/compile.ts | 95 ++---------- lib/src/sync-compiler.ts | 307 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 308 insertions(+), 94 deletions(-) diff --git a/lib/src/compile.ts b/lib/src/compile.ts index 96d3a0a2..833b1323 100644 --- a/lib/src/compile.ts +++ b/lib/src/compile.ts @@ -16,7 +16,11 @@ import {FunctionRegistry} from './function-registry'; import {ImporterRegistry} from './importer-registry'; import {MessageTransformer} from './message-transformer'; import {PacketTransformer} from './packet-transformer'; -import {SyncEmbeddedCompiler} from './sync-compiler'; +import { + initCompiler, + OptionsWithLegacy, + StringOptionsWithLegacy, +} from './sync-compiler'; import {deprotofySourceSpan} from './deprotofy-span'; import { removeLegacyImporter, @@ -24,45 +28,24 @@ import { legacyImporterProtocol, } from './legacy/utils'; -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. -type OptionsWithLegacy = Options & { - legacy?: boolean; -}; - -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. -type StringOptionsWithLegacy = - StringOptions & {legacy?: boolean}; - export function compile( path: string, options?: OptionsWithLegacy<'sync'> ): CompileResult { - const importers = new ImporterRegistry(options); - return compileRequestSync( - newCompilePathRequest(path, importers, options), - importers, - options - ); + const compiler = initCompiler(); + const result = compiler.compile(path, options); + compiler.dispose(); + return result; } export function compileString( source: string, options?: StringOptionsWithLegacy<'sync'> ): CompileResult { - const importers = new ImporterRegistry(options); - return compileRequestSync( - newCompileStringRequest(source, importers, options), - importers, - options - ); + const compiler = initCompiler(); + const result = compiler.compileString(source, options); + compiler.dispose(); + return result; } export function compileAsync( @@ -212,58 +195,6 @@ async function compileRequestAsync( } } -// Spins up a compiler, then sends it a compile request. Returns a promise that -// resolves with the CompileResult. Throws if there were any protocol or -// compilation errors. Shuts down the compiler after compilation. -function compileRequestSync( - request: proto.InboundMessage_CompileRequest, - importers: ImporterRegistry<'sync'>, - options?: OptionsWithLegacy<'sync'> -): CompileResult { - const functions = new FunctionRegistry(options?.functions); - const embeddedCompiler = new SyncEmbeddedCompiler(); - embeddedCompiler.stderr$.subscribe(data => process.stderr.write(data)); - - try { - const dispatcher = createDispatcher<'sync'>( - embeddedCompiler.stdout$, - buffer => { - embeddedCompiler.writeStdin(buffer); - }, - { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - } - ); - - dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); - - let error: unknown; - let response: proto.OutboundMessage_CompileResponse | undefined; - dispatcher.sendCompileRequest(request, (error_, response_) => { - if (error_) { - error = error_; - } else { - response = response_; - } - }); - - for (;;) { - if (!embeddedCompiler.yield()) { - throw utils.compilerError('Embedded compiler exited unexpectedly.'); - } - - if (error) throw error; - if (response) return handleCompileResponse(response); - } - } finally { - embeddedCompiler.close(); - embeddedCompiler.yieldUntilExit(); - } -} - /** * Creates a dispatcher that dispatches messages from the given `stdout` stream. */ diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 2950fbdc..b85f6307 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -2,16 +2,47 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import {Subject} from 'rxjs'; +import { Observable, Subject } from 'rxjs'; -import {SyncProcess} from './sync-process'; -import {compilerCommand} from './compiler-path'; +import * as p from 'path'; +import * as supportsColor from 'supports-color'; +import { compilerCommand } from './compiler-path'; +import { deprotofySourceSpan } from './deprotofy-span'; +import { Dispatcher, DispatcherHandlers } from './dispatcher'; +import { Exception } from './exception'; +import { FunctionRegistry } from './function-registry'; +import { ImporterRegistry } from './importer-registry'; +import { legacyImporterProtocol } from './legacy/utils'; +import { MessageTransformer } from './message-transformer'; +import { PacketTransformer } from './packet-transformer'; +import { SyncProcess } from './sync-process'; +import * as utils from './utils'; +import * as proto from './vendor/embedded_sass_pb'; +import { SourceSpan } from './vendor/sass'; +import { CompileResult } from './vendor/sass/compile'; +import { Options, StringOptions } from './vendor/sass/options'; + +/// Allow the legacy API to pass in an option signaling to the modern API that +/// it's being run in legacy mode. +/// +/// This is not intended for API users to pass in, and may be broken without +/// warning in the future. +export type OptionsWithLegacy = Options & { + legacy?: boolean; +}; + +/// Allow the legacy API to pass in an option signaling to the modern API that +/// it's being run in legacy mode. +/// +/// This is not intended for API users to pass in, and may be broken without +/// warning in the future. +export type StringOptionsWithLegacy = + StringOptions & {legacy?: boolean}; /** - * A synchronous wrapper for the embedded Sass compiler that exposes its stdio - * streams as Observables. + * A synchronous wrapper for the embedded Sass compiler */ -export class SyncEmbeddedCompiler { +export class Compiler { /** The underlying process that's being wrapped. */ private readonly process = new SyncProcess( compilerCommand[0], @@ -20,20 +51,20 @@ export class SyncEmbeddedCompiler { ); /** The buffers emitted by the child process's stdout. */ - readonly stdout$ = new Subject(); + private readonly stdout$ = new Subject(); /** The buffers emitted by the child process's stderr. */ - readonly stderr$ = new Subject(); + private readonly stderr$ = new Subject(); /** Whether the underlying compiler has already exited. */ private exited = false; /** Writes `buffer` to the child process's stdin. */ - writeStdin(buffer: Buffer): void { + private writeStdin(buffer: Buffer): void { this.process.stdin.write(buffer); } - yield(): boolean { + private yield(): boolean { const event = this.process.yield(); switch (event.type) { case 'stdout': @@ -51,14 +82,266 @@ export class SyncEmbeddedCompiler { } /** Blocks until the underlying process exits. */ - yieldUntilExit(): void { + private yieldUntilExit(): void { while (!this.exited) { this.yield(); } } + /** + * Creates a dispatcher that dispatches messages from the given `stdout` stream. + */ + private createDispatcher( + stdout: Observable, + writeStdin: (buffer: Buffer) => void, + handlers: DispatcherHandlers + ): Dispatcher { + const packetTransformer = new PacketTransformer(stdout, writeStdin); + + const messageTransformer = new MessageTransformer( + packetTransformer.outboundProtobufs$, + packet => packetTransformer.writeInboundProtobuf(packet) + ); + + return new Dispatcher( + // Since we only use one compilation per process, we can get away with + // hardcoding a compilation ID. Once we support multiple concurrent + // compilations with the same process, we'll need to ensure each one uses a + // unique ID. + 1, + messageTransformer.outboundMessages$, + message => messageTransformer.writeInboundMessage(message), + handlers + ); + } + + // Creates a compilation request for the given `options` without adding any + // input-specific options. + private newCompileRequest( + importers: ImporterRegistry<'sync' | 'async'>, + options?: Options<'sync' | 'async'> + ): proto.InboundMessage_CompileRequest { + const request = new proto.InboundMessage_CompileRequest({ + importers: importers.importers, + globalFunctions: Object.keys(options?.functions ?? {}), + sourceMap: !!options?.sourceMap, + sourceMapIncludeSources: !!options?.sourceMapIncludeSources, + alertColor: options?.alertColor ?? !!supportsColor.stdout, + alertAscii: !!options?.alertAscii, + quietDeps: !!options?.quietDeps, + verbose: !!options?.verbose, + charset: !!(options?.charset ?? true), + }); + + switch (options?.style ?? 'expanded') { + case 'expanded': + request.style = proto.OutputStyle.EXPANDED; + break; + + case 'compressed': + request.style = proto.OutputStyle.COMPRESSED; + break; + + default: + throw new Error(`Unknown options.style: "${options?.style}"`); + } + + return request; + } + + // Creates a request for compiling a file. + private newCompilePathRequest( + path: string, + importers: ImporterRegistry<'sync' | 'async'>, + options?: Options<'sync' | 'async'> + ): proto.InboundMessage_CompileRequest { + const request = this.newCompileRequest(importers, options); + request.input = {case: 'path', value: path}; + return request; + } + + // Creates a request for compiling a string. + private newCompileStringRequest( + source: string, + importers: ImporterRegistry<'sync' | 'async'>, + options?: StringOptions<'sync' | 'async'> + ): proto.InboundMessage_CompileRequest { + const input = new proto.InboundMessage_CompileRequest_StringInput({ + source, + syntax: utils.protofySyntax(options?.syntax ?? 'scss'), + }); + + const url = options?.url?.toString(); + if (url && url !== legacyImporterProtocol) { + input.url = url; + } + + if (options && 'importer' in options && options.importer) { + input.importer = importers.register(options.importer); + } else if (url === legacyImporterProtocol) { + input.importer = new proto.InboundMessage_CompileRequest_Importer({ + importer: {case: 'path', value: p.resolve('.')}, + }); + } else { + // When importer is not set on the host, the compiler will set a + // FileSystemImporter if `url` is set to a file: url or a NoOpImporter. + } + + const request = this.newCompileRequest(importers, options); + request.input = {case: 'string', value: input}; + return request; + } + + /** Handles a log event according to `options`. */ + private handleLogEvent( + options: OptionsWithLegacy<'sync' | 'async'> | undefined, + event: proto.OutboundMessage_LogEvent + ): void { + let span = event.span ? deprotofySourceSpan(event.span) : null; + let message = event.message; + let formatted = event.formatted; + + if (event.type === proto.LogEventType.DEBUG) { + if (options?.logger?.debug) { + options.logger.debug(message, { + span: span!, + }); + } else { + console.error(formatted); + } + } else { + if (options?.logger?.warn) { + const params: { + deprecation: boolean; + span?: SourceSpan; + stack?: string; + } = { + deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, + }; + if (span) params.span = span; + + const stack = event.stackTrace; + if (stack) { + params.stack = stack; + } + + options.logger.warn(message, params); + } else { + console.error(formatted); + } + } + } + + /** + * Converts a `CompileResponse` into a `CompileResult`. + * + * Throws a `SassException` if the compilation failed. + */ + private handleCompileResponse( + response: proto.OutboundMessage_CompileResponse + ): CompileResult { + if (response.result.case === 'success') { + const success = response.result.value; + const result: CompileResult = { + css: success.css, + loadedUrls: response.loadedUrls.map(url => new URL(url)), + }; + + const sourceMap = success.sourceMap; + if (sourceMap) result.sourceMap = JSON.parse(sourceMap); + return result; + } else if (response.result.case === 'failure') { + throw new Exception(response.result.value); + } else { + throw utils.compilerError('Compiler sent empty CompileResponse.'); + } + } + + // Spins up a compiler, then sends it a compile request. Returns a promise that + // resolves with the CompileResult. Throws if there were any protocol or + // compilation errors. Shuts down the compiler after compilation. + private compileRequestSync( + request: proto.InboundMessage_CompileRequest, + importers: ImporterRegistry<'sync'>, + options?: OptionsWithLegacy<'sync'> + ): CompileResult { + const functions = new FunctionRegistry(options?.functions); + this.stderr$.subscribe(data => process.stderr.write(data)); + + try { + const dispatcher = this.createDispatcher<'sync'>( + this.stdout$, + buffer => { + this.writeStdin(buffer); + }, + { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + } + ); + + dispatcher.logEvents$.subscribe(event => + this.handleLogEvent(options, event) + ); + + let error: unknown; + let response: proto.OutboundMessage_CompileResponse | undefined; + dispatcher.sendCompileRequest(request, (error_, response_) => { + if (error_) { + error = error_; + } else { + response = response_; + } + }); + + for (;;) { + if (!this.yield()) { + throw utils.compilerError('Embedded compiler exited unexpectedly.'); + } + + if (error) throw error; + if (response) return this.handleCompileResponse(response); + } + } finally { + this.dispose(); + this.yieldUntilExit(); + } + } + + private throwIfClosed(): void { + if (this.exited) { + throw utils.compilerError('Sync compiler has already exited.'); + } + } + + compile(path: string, options?: Options<'sync'>): CompileResult { + this.throwIfClosed(); + const importers = new ImporterRegistry(options); + return this.compileRequestSync( + this.newCompilePathRequest(path, importers, options), + importers, + options + ); + } + + compileString(source: string, options?: Options<'sync'>): CompileResult { + this.throwIfClosed(); + const importers = new ImporterRegistry(options); + return this.compileRequestSync( + this.newCompileStringRequest(source, importers, options), + importers, + options + ); + } + /** Kills the child process, cleaning up all associated Observables. */ - close() { + dispose() { this.process.stdin.end(); } } + +export function initCompiler(): Compiler { + return new Compiler(); +} From eb2320496b63e563bb1724dec3cebc424ff08115 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Mon, 13 Nov 2023 23:06:53 +0000 Subject: [PATCH 02/35] Factor out common compilation helpers --- lib/src/compile.ts | 21 ++-- lib/src/compiler.ts | 206 ++++++++++++++++++++++++++++++++++ lib/src/sync-compiler.ts | 231 ++++----------------------------------- 3 files changed, 236 insertions(+), 222 deletions(-) create mode 100644 lib/src/compiler.ts diff --git a/lib/src/compile.ts b/lib/src/compile.ts index 833b1323..287b8835 100644 --- a/lib/src/compile.ts +++ b/lib/src/compile.ts @@ -6,27 +6,24 @@ import * as p from 'path'; import {Observable} from 'rxjs'; import * as supportsColor from 'supports-color'; -import * as proto from './vendor/embedded_sass_pb'; -import * as utils from './utils'; import {AsyncEmbeddedCompiler} from './async-compiler'; -import {CompileResult, Options, SourceSpan, StringOptions} from './vendor/sass'; +import {OptionsWithLegacy, StringOptionsWithLegacy} from './compiler'; +import {deprotofySourceSpan} from './deprotofy-span'; import {Dispatcher, DispatcherHandlers} from './dispatcher'; import {Exception} from './exception'; import {FunctionRegistry} from './function-registry'; import {ImporterRegistry} from './importer-registry'; -import {MessageTransformer} from './message-transformer'; -import {PacketTransformer} from './packet-transformer'; -import { - initCompiler, - OptionsWithLegacy, - StringOptionsWithLegacy, -} from './sync-compiler'; -import {deprotofySourceSpan} from './deprotofy-span'; import { + legacyImporterProtocol, removeLegacyImporter, removeLegacyImporterFromSpan, - legacyImporterProtocol, } from './legacy/utils'; +import {MessageTransformer} from './message-transformer'; +import {PacketTransformer} from './packet-transformer'; +import {initCompiler} from './sync-compiler'; +import * as utils from './utils'; +import * as proto from './vendor/embedded_sass_pb'; +import {CompileResult, Options, SourceSpan, StringOptions} from './vendor/sass'; export function compile( path: string, diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts new file mode 100644 index 00000000..d86c3d62 --- /dev/null +++ b/lib/src/compiler.ts @@ -0,0 +1,206 @@ +// Copyright 2021 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import { Observable } from 'rxjs'; + +import * as p from 'path'; +import * as supportsColor from 'supports-color'; +import { deprotofySourceSpan } from './deprotofy-span'; +import { Dispatcher, DispatcherHandlers } from './dispatcher'; +import { Exception } from './exception'; +import { ImporterRegistry } from './importer-registry'; +import { legacyImporterProtocol } from './legacy/utils'; +import { MessageTransformer } from './message-transformer'; +import { PacketTransformer } from './packet-transformer'; +import * as utils from './utils'; +import * as proto from './vendor/embedded_sass_pb'; +import { SourceSpan } from './vendor/sass'; +import { CompileResult } from './vendor/sass/compile'; +import { Options, StringOptions } from './vendor/sass/options'; + +/// Allow the legacy API to pass in an option signaling to the modern API that +/// it's being run in legacy mode. +/// +/// This is not intended for API users to pass in, and may be broken without +/// warning in the future. +export type OptionsWithLegacy = Options & { + legacy?: boolean; +}; + +/// Allow the legacy API to pass in an option signaling to the modern API that +/// it's being run in legacy mode. +/// +/// This is not intended for API users to pass in, and may be broken without +/// warning in the future. +export type StringOptionsWithLegacy = + StringOptions & {legacy?: boolean}; + +/** + * Creates a dispatcher that dispatches messages from the given `stdout` stream. + */ +export function createDispatcher( + stdout: Observable, + writeStdin: (buffer: Buffer) => void, + handlers: DispatcherHandlers +): Dispatcher { + const packetTransformer = new PacketTransformer(stdout, writeStdin); + + const messageTransformer = new MessageTransformer( + packetTransformer.outboundProtobufs$, + packet => packetTransformer.writeInboundProtobuf(packet) + ); + + return new Dispatcher( + // Since we only use one compilation per process, we can get away with + // hardcoding a compilation ID. Once we support multiple concurrent + // compilations with the same process, we'll need to ensure each one uses a + // unique ID. + 1, + messageTransformer.outboundMessages$, + message => messageTransformer.writeInboundMessage(message), + handlers + ); +} + +// Creates a compilation request for the given `options` without adding any +// input-specific options. +function newCompileRequest( + importers: ImporterRegistry<'sync' | 'async'>, + options?: Options<'sync' | 'async'> +): proto.InboundMessage_CompileRequest { + const request = new proto.InboundMessage_CompileRequest({ + importers: importers.importers, + globalFunctions: Object.keys(options?.functions ?? {}), + sourceMap: !!options?.sourceMap, + sourceMapIncludeSources: !!options?.sourceMapIncludeSources, + alertColor: options?.alertColor ?? !!supportsColor.stdout, + alertAscii: !!options?.alertAscii, + quietDeps: !!options?.quietDeps, + verbose: !!options?.verbose, + charset: !!(options?.charset ?? true), + }); + + switch (options?.style ?? 'expanded') { + case 'expanded': + request.style = proto.OutputStyle.EXPANDED; + break; + + case 'compressed': + request.style = proto.OutputStyle.COMPRESSED; + break; + + default: + throw new Error(`Unknown options.style: "${options?.style}"`); + } + + return request; +} + +// Creates a request for compiling a file. +export function newCompilePathRequest( + path: string, + importers: ImporterRegistry<'sync' | 'async'>, + options?: Options<'sync' | 'async'> +): proto.InboundMessage_CompileRequest { + const request = newCompileRequest(importers, options); + request.input = {case: 'path', value: path}; + return request; +} + +// Creates a request for compiling a string. +export function newCompileStringRequest( + source: string, + importers: ImporterRegistry<'sync' | 'async'>, + options?: StringOptions<'sync' | 'async'> +): proto.InboundMessage_CompileRequest { + const input = new proto.InboundMessage_CompileRequest_StringInput({ + source, + syntax: utils.protofySyntax(options?.syntax ?? 'scss'), + }); + + const url = options?.url?.toString(); + if (url && url !== legacyImporterProtocol) { + input.url = url; + } + + if (options && 'importer' in options && options.importer) { + input.importer = importers.register(options.importer); + } else if (url === legacyImporterProtocol) { + input.importer = new proto.InboundMessage_CompileRequest_Importer({ + importer: {case: 'path', value: p.resolve('.')}, + }); + } else { + // When importer is not set on the host, the compiler will set a + // FileSystemImporter if `url` is set to a file: url or a NoOpImporter. + } + + const request = newCompileRequest(importers, options); + request.input = {case: 'string', value: input}; + return request; +} + +/** Handles a log event according to `options`. */ +export function handleLogEvent( + options: OptionsWithLegacy<'sync' | 'async'> | undefined, + event: proto.OutboundMessage_LogEvent +): void { + let span = event.span ? deprotofySourceSpan(event.span) : null; + let message = event.message; + let formatted = event.formatted; + + if (event.type === proto.LogEventType.DEBUG) { + if (options?.logger?.debug) { + options.logger.debug(message, { + span: span!, + }); + } else { + console.error(formatted); + } + } else { + if (options?.logger?.warn) { + const params: { + deprecation: boolean; + span?: SourceSpan; + stack?: string; + } = { + deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, + }; + if (span) params.span = span; + + const stack = event.stackTrace; + if (stack) { + params.stack = stack; + } + + options.logger.warn(message, params); + } else { + console.error(formatted); + } + } +} + +/** + * Converts a `CompileResponse` into a `CompileResult`. + * + * Throws a `SassException` if the compilation failed. + */ +export function handleCompileResponse( + response: proto.OutboundMessage_CompileResponse +): CompileResult { + if (response.result.case === 'success') { + const success = response.result.value; + const result: CompileResult = { + css: success.css, + loadedUrls: response.loadedUrls.map(url => new URL(url)), + }; + + const sourceMap = success.sourceMap; + if (sourceMap) result.sourceMap = JSON.parse(sourceMap); + return result; + } else if (response.result.case === 'failure') { + throw new Exception(response.result.value); + } else { + throw utils.compilerError('Compiler sent empty CompileResponse.'); + } +} diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index b85f6307..08475ddc 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -2,42 +2,24 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import { Observable, Subject } from 'rxjs'; - -import * as p from 'path'; -import * as supportsColor from 'supports-color'; -import { compilerCommand } from './compiler-path'; -import { deprotofySourceSpan } from './deprotofy-span'; -import { Dispatcher, DispatcherHandlers } from './dispatcher'; -import { Exception } from './exception'; -import { FunctionRegistry } from './function-registry'; -import { ImporterRegistry } from './importer-registry'; -import { legacyImporterProtocol } from './legacy/utils'; -import { MessageTransformer } from './message-transformer'; -import { PacketTransformer } from './packet-transformer'; -import { SyncProcess } from './sync-process'; +import {Subject} from 'rxjs'; + +import { + OptionsWithLegacy, + createDispatcher, + handleCompileResponse, + handleLogEvent, + newCompilePathRequest, + newCompileStringRequest, +} from './compiler'; +import {compilerCommand} from './compiler-path'; +import {FunctionRegistry} from './function-registry'; +import {ImporterRegistry} from './importer-registry'; +import {SyncProcess} from './sync-process'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; -import { SourceSpan } from './vendor/sass'; -import { CompileResult } from './vendor/sass/compile'; -import { Options, StringOptions } from './vendor/sass/options'; - -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. -export type OptionsWithLegacy = Options & { - legacy?: boolean; -}; - -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. -export type StringOptionsWithLegacy = - StringOptions & {legacy?: boolean}; +import {CompileResult} from './vendor/sass/compile'; +import {Options} from './vendor/sass/options'; /** * A synchronous wrapper for the embedded Sass compiler @@ -88,175 +70,6 @@ export class Compiler { } } - /** - * Creates a dispatcher that dispatches messages from the given `stdout` stream. - */ - private createDispatcher( - stdout: Observable, - writeStdin: (buffer: Buffer) => void, - handlers: DispatcherHandlers - ): Dispatcher { - const packetTransformer = new PacketTransformer(stdout, writeStdin); - - const messageTransformer = new MessageTransformer( - packetTransformer.outboundProtobufs$, - packet => packetTransformer.writeInboundProtobuf(packet) - ); - - return new Dispatcher( - // Since we only use one compilation per process, we can get away with - // hardcoding a compilation ID. Once we support multiple concurrent - // compilations with the same process, we'll need to ensure each one uses a - // unique ID. - 1, - messageTransformer.outboundMessages$, - message => messageTransformer.writeInboundMessage(message), - handlers - ); - } - - // Creates a compilation request for the given `options` without adding any - // input-specific options. - private newCompileRequest( - importers: ImporterRegistry<'sync' | 'async'>, - options?: Options<'sync' | 'async'> - ): proto.InboundMessage_CompileRequest { - const request = new proto.InboundMessage_CompileRequest({ - importers: importers.importers, - globalFunctions: Object.keys(options?.functions ?? {}), - sourceMap: !!options?.sourceMap, - sourceMapIncludeSources: !!options?.sourceMapIncludeSources, - alertColor: options?.alertColor ?? !!supportsColor.stdout, - alertAscii: !!options?.alertAscii, - quietDeps: !!options?.quietDeps, - verbose: !!options?.verbose, - charset: !!(options?.charset ?? true), - }); - - switch (options?.style ?? 'expanded') { - case 'expanded': - request.style = proto.OutputStyle.EXPANDED; - break; - - case 'compressed': - request.style = proto.OutputStyle.COMPRESSED; - break; - - default: - throw new Error(`Unknown options.style: "${options?.style}"`); - } - - return request; - } - - // Creates a request for compiling a file. - private newCompilePathRequest( - path: string, - importers: ImporterRegistry<'sync' | 'async'>, - options?: Options<'sync' | 'async'> - ): proto.InboundMessage_CompileRequest { - const request = this.newCompileRequest(importers, options); - request.input = {case: 'path', value: path}; - return request; - } - - // Creates a request for compiling a string. - private newCompileStringRequest( - source: string, - importers: ImporterRegistry<'sync' | 'async'>, - options?: StringOptions<'sync' | 'async'> - ): proto.InboundMessage_CompileRequest { - const input = new proto.InboundMessage_CompileRequest_StringInput({ - source, - syntax: utils.protofySyntax(options?.syntax ?? 'scss'), - }); - - const url = options?.url?.toString(); - if (url && url !== legacyImporterProtocol) { - input.url = url; - } - - if (options && 'importer' in options && options.importer) { - input.importer = importers.register(options.importer); - } else if (url === legacyImporterProtocol) { - input.importer = new proto.InboundMessage_CompileRequest_Importer({ - importer: {case: 'path', value: p.resolve('.')}, - }); - } else { - // When importer is not set on the host, the compiler will set a - // FileSystemImporter if `url` is set to a file: url or a NoOpImporter. - } - - const request = this.newCompileRequest(importers, options); - request.input = {case: 'string', value: input}; - return request; - } - - /** Handles a log event according to `options`. */ - private handleLogEvent( - options: OptionsWithLegacy<'sync' | 'async'> | undefined, - event: proto.OutboundMessage_LogEvent - ): void { - let span = event.span ? deprotofySourceSpan(event.span) : null; - let message = event.message; - let formatted = event.formatted; - - if (event.type === proto.LogEventType.DEBUG) { - if (options?.logger?.debug) { - options.logger.debug(message, { - span: span!, - }); - } else { - console.error(formatted); - } - } else { - if (options?.logger?.warn) { - const params: { - deprecation: boolean; - span?: SourceSpan; - stack?: string; - } = { - deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, - }; - if (span) params.span = span; - - const stack = event.stackTrace; - if (stack) { - params.stack = stack; - } - - options.logger.warn(message, params); - } else { - console.error(formatted); - } - } - } - - /** - * Converts a `CompileResponse` into a `CompileResult`. - * - * Throws a `SassException` if the compilation failed. - */ - private handleCompileResponse( - response: proto.OutboundMessage_CompileResponse - ): CompileResult { - if (response.result.case === 'success') { - const success = response.result.value; - const result: CompileResult = { - css: success.css, - loadedUrls: response.loadedUrls.map(url => new URL(url)), - }; - - const sourceMap = success.sourceMap; - if (sourceMap) result.sourceMap = JSON.parse(sourceMap); - return result; - } else if (response.result.case === 'failure') { - throw new Exception(response.result.value); - } else { - throw utils.compilerError('Compiler sent empty CompileResponse.'); - } - } - // Spins up a compiler, then sends it a compile request. Returns a promise that // resolves with the CompileResult. Throws if there were any protocol or // compilation errors. Shuts down the compiler after compilation. @@ -269,7 +82,7 @@ export class Compiler { this.stderr$.subscribe(data => process.stderr.write(data)); try { - const dispatcher = this.createDispatcher<'sync'>( + const dispatcher = createDispatcher<'sync'>( this.stdout$, buffer => { this.writeStdin(buffer); @@ -282,9 +95,7 @@ export class Compiler { } ); - dispatcher.logEvents$.subscribe(event => - this.handleLogEvent(options, event) - ); + dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); let error: unknown; let response: proto.OutboundMessage_CompileResponse | undefined; @@ -302,7 +113,7 @@ export class Compiler { } if (error) throw error; - if (response) return this.handleCompileResponse(response); + if (response) return handleCompileResponse(response); } } finally { this.dispose(); @@ -320,7 +131,7 @@ export class Compiler { this.throwIfClosed(); const importers = new ImporterRegistry(options); return this.compileRequestSync( - this.newCompilePathRequest(path, importers, options), + newCompilePathRequest(path, importers, options), importers, options ); @@ -330,7 +141,7 @@ export class Compiler { this.throwIfClosed(); const importers = new ImporterRegistry(options); return this.compileRequestSync( - this.newCompileStringRequest(source, importers, options), + newCompileStringRequest(source, importers, options), importers, options ); From 9e78051f7118619a71aac5b90af751fccfe9f5e5 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Mon, 13 Nov 2023 23:37:36 +0000 Subject: [PATCH 03/35] Add AsyncCompiler --- lib/src/async-compiler.ts | 110 ++++++++++++++-- lib/src/compile.ts | 259 ++------------------------------------ lib/src/sync-compiler.ts | 14 +-- 3 files changed, 122 insertions(+), 261 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 90aa0c88..e66f1deb 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -6,13 +6,26 @@ import {spawn} from 'child_process'; import {Observable} from 'rxjs'; import {takeUntil} from 'rxjs/operators'; +import { + OptionsWithLegacy, + StringOptionsWithLegacy, + createDispatcher, + handleCompileResponse, + handleLogEvent, + newCompilePathRequest, + newCompileStringRequest, +} from './compiler'; import {compilerCommand} from './compiler-path'; +import {FunctionRegistry} from './function-registry'; +import {ImporterRegistry} from './importer-registry'; +import * as utils from './utils'; +import * as proto from './vendor/embedded_sass_pb'; +import {CompileResult} from './vendor/sass'; /** - * An asynchronous wrapper for the embedded Sass compiler that exposes its stdio - * streams as Observables. + * An asynchronous wrapper for the embedded Sass compiler */ -export class AsyncEmbeddedCompiler { +export class AsyncCompiler { /** The underlying process that's being wrapped. */ private readonly process = spawn( compilerCommand[0], @@ -20,28 +33,109 @@ export class AsyncEmbeddedCompiler { {windowsHide: true} ); + /** Whether the underlying compiler has already exited. */ + private disposed = false; + /** The child process's exit event. */ - readonly exit$ = new Promise(resolve => { + private readonly exit$ = new Promise(resolve => { this.process.on('exit', code => resolve(code)); }); /** The buffers emitted by the child process's stdout. */ - readonly stdout$ = new Observable(observer => { + private readonly stdout$ = new Observable(observer => { this.process.stdout.on('data', buffer => observer.next(buffer)); }).pipe(takeUntil(this.exit$)); /** The buffers emitted by the child process's stderr. */ - readonly stderr$ = new Observable(observer => { + private readonly stderr$ = new Observable(observer => { this.process.stderr.on('data', buffer => observer.next(buffer)); }).pipe(takeUntil(this.exit$)); /** Writes `buffer` to the child process's stdin. */ - writeStdin(buffer: Buffer): void { + private writeStdin(buffer: Buffer): void { this.process.stdin.write(buffer); } + private throwIfDisposed(): void { + if (this.disposed) { + throw utils.compilerError('Sync compiler has already exited.'); + } + } + + // Spins up a compiler, then sends it a compile request. Returns a promise that + // resolves with the CompileResult. Throws if there were any protocol or + // compilation errors. Shuts down the compiler after compilation. + private async compileRequestAsync( + request: proto.InboundMessage_CompileRequest, + importers: ImporterRegistry<'async'>, + options?: OptionsWithLegacy<'async'> & {legacy?: boolean} + ): Promise { + const functions = new FunctionRegistry(options?.functions); + this.stderr$.subscribe(data => process.stderr.write(data)); + + const dispatcher = createDispatcher<'async'>( + this.stdout$, + buffer => { + this.writeStdin(buffer); + }, + { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + } + ); + + dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); + + return handleCompileResponse( + await new Promise( + (resolve, reject) => + dispatcher.sendCompileRequest(request, (err, response) => { + if (err) { + reject(err); + } else { + resolve(response!); + } + }) + ) + ); + } + + compileAsync( + path: string, + options?: OptionsWithLegacy<'async'> + ): Promise { + this.throwIfDisposed(); + const importers = new ImporterRegistry(options); + return this.compileRequestAsync( + newCompilePathRequest(path, importers, options), + importers, + options + ); + } + + compileStringAsync( + source: string, + options?: StringOptionsWithLegacy<'async'> + ): Promise { + this.throwIfDisposed(); + const importers = new ImporterRegistry(options); + return this.compileRequestAsync( + newCompileStringRequest(source, importers, options), + importers, + options + ); + } + /** Kills the child process, cleaning up all associated Observables. */ - close() { + async dispose() { + this.disposed = true; this.process.stdin.end(); + await this.exit$; } } + +export async function initAsyncCompiler(): Promise { + return new AsyncCompiler(); +} diff --git a/lib/src/compile.ts b/lib/src/compile.ts index 287b8835..9a340b21 100644 --- a/lib/src/compile.ts +++ b/lib/src/compile.ts @@ -2,28 +2,10 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import * as p from 'path'; -import {Observable} from 'rxjs'; -import * as supportsColor from 'supports-color'; - -import {AsyncEmbeddedCompiler} from './async-compiler'; +import {initAsyncCompiler} from './async-compiler'; import {OptionsWithLegacy, StringOptionsWithLegacy} from './compiler'; -import {deprotofySourceSpan} from './deprotofy-span'; -import {Dispatcher, DispatcherHandlers} from './dispatcher'; -import {Exception} from './exception'; -import {FunctionRegistry} from './function-registry'; -import {ImporterRegistry} from './importer-registry'; -import { - legacyImporterProtocol, - removeLegacyImporter, - removeLegacyImporterFromSpan, -} from './legacy/utils'; -import {MessageTransformer} from './message-transformer'; -import {PacketTransformer} from './packet-transformer'; import {initCompiler} from './sync-compiler'; -import * as utils from './utils'; -import * as proto from './vendor/embedded_sass_pb'; -import {CompileResult, Options, SourceSpan, StringOptions} from './vendor/sass'; +import {CompileResult} from './vendor/sass'; export function compile( path: string, @@ -45,241 +27,26 @@ export function compileString( return result; } -export function compileAsync( +export async function compileAsync( path: string, options?: OptionsWithLegacy<'async'> ): Promise { - const importers = new ImporterRegistry(options); - return compileRequestAsync( - newCompilePathRequest(path, importers, options), - importers, - options - ); + const compiler = await initAsyncCompiler(); + try { + return await compiler.compileAsync(path, options); + } finally { + await compiler.dispose(); + } } -export function compileStringAsync( +export async function compileStringAsync( source: string, options?: StringOptionsWithLegacy<'async'> ): Promise { - const importers = new ImporterRegistry(options); - return compileRequestAsync( - newCompileStringRequest(source, importers, options), - importers, - options - ); -} - -// Creates a request for compiling a file. -function newCompilePathRequest( - path: string, - importers: ImporterRegistry<'sync' | 'async'>, - options?: Options<'sync' | 'async'> -): proto.InboundMessage_CompileRequest { - const request = newCompileRequest(importers, options); - request.input = {case: 'path', value: path}; - return request; -} - -// Creates a request for compiling a string. -function newCompileStringRequest( - source: string, - importers: ImporterRegistry<'sync' | 'async'>, - options?: StringOptions<'sync' | 'async'> -): proto.InboundMessage_CompileRequest { - const input = new proto.InboundMessage_CompileRequest_StringInput({ - source, - syntax: utils.protofySyntax(options?.syntax ?? 'scss'), - }); - - const url = options?.url?.toString(); - if (url && url !== legacyImporterProtocol) { - input.url = url; - } - - if (options && 'importer' in options && options.importer) { - input.importer = importers.register(options.importer); - } else if (url === legacyImporterProtocol) { - input.importer = new proto.InboundMessage_CompileRequest_Importer({ - importer: {case: 'path', value: p.resolve('.')}, - }); - } else { - // When importer is not set on the host, the compiler will set a - // FileSystemImporter if `url` is set to a file: url or a NoOpImporter. - } - - const request = newCompileRequest(importers, options); - request.input = {case: 'string', value: input}; - return request; -} - -// Creates a compilation request for the given `options` without adding any -// input-specific options. -function newCompileRequest( - importers: ImporterRegistry<'sync' | 'async'>, - options?: Options<'sync' | 'async'> -): proto.InboundMessage_CompileRequest { - const request = new proto.InboundMessage_CompileRequest({ - importers: importers.importers, - globalFunctions: Object.keys(options?.functions ?? {}), - sourceMap: !!options?.sourceMap, - sourceMapIncludeSources: !!options?.sourceMapIncludeSources, - alertColor: options?.alertColor ?? !!supportsColor.stdout, - alertAscii: !!options?.alertAscii, - quietDeps: !!options?.quietDeps, - verbose: !!options?.verbose, - charset: !!(options?.charset ?? true), - }); - - switch (options?.style ?? 'expanded') { - case 'expanded': - request.style = proto.OutputStyle.EXPANDED; - break; - - case 'compressed': - request.style = proto.OutputStyle.COMPRESSED; - break; - - default: - throw new Error(`Unknown options.style: "${options?.style}"`); - } - - return request; -} - -// Spins up a compiler, then sends it a compile request. Returns a promise that -// resolves with the CompileResult. Throws if there were any protocol or -// compilation errors. Shuts down the compiler after compilation. -async function compileRequestAsync( - request: proto.InboundMessage_CompileRequest, - importers: ImporterRegistry<'async'>, - options?: OptionsWithLegacy<'async'> & {legacy?: boolean} -): Promise { - const functions = new FunctionRegistry(options?.functions); - const embeddedCompiler = new AsyncEmbeddedCompiler(); - embeddedCompiler.stderr$.subscribe(data => process.stderr.write(data)); - + const compiler = await initAsyncCompiler(); try { - const dispatcher = createDispatcher<'async'>( - embeddedCompiler.stdout$, - buffer => { - embeddedCompiler.writeStdin(buffer); - }, - { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - } - ); - - dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); - - return handleCompileResponse( - await new Promise( - (resolve, reject) => - dispatcher.sendCompileRequest(request, (err, response) => { - if (err) { - reject(err); - } else { - resolve(response!); - } - }) - ) - ); + return await compiler.compileStringAsync(source, options); } finally { - embeddedCompiler.close(); - await embeddedCompiler.exit$; - } -} - -/** - * Creates a dispatcher that dispatches messages from the given `stdout` stream. - */ -function createDispatcher( - stdout: Observable, - writeStdin: (buffer: Buffer) => void, - handlers: DispatcherHandlers -): Dispatcher { - const packetTransformer = new PacketTransformer(stdout, writeStdin); - - const messageTransformer = new MessageTransformer( - packetTransformer.outboundProtobufs$, - packet => packetTransformer.writeInboundProtobuf(packet) - ); - - return new Dispatcher( - // Since we only use one compilation per process, we can get away with - // hardcoding a compilation ID. Once we support multiple concurrent - // compilations with the same process, we'll need to ensure each one uses a - // unique ID. - 1, - messageTransformer.outboundMessages$, - message => messageTransformer.writeInboundMessage(message), - handlers - ); -} - -/** Handles a log event according to `options`. */ -function handleLogEvent( - options: OptionsWithLegacy<'sync' | 'async'> | undefined, - event: proto.OutboundMessage_LogEvent -): void { - let span = event.span ? deprotofySourceSpan(event.span) : null; - if (span && options?.legacy) span = removeLegacyImporterFromSpan(span); - let message = event.message; - if (options?.legacy) message = removeLegacyImporter(message); - let formatted = event.formatted; - if (options?.legacy) formatted = removeLegacyImporter(formatted); - - if (event.type === proto.LogEventType.DEBUG) { - if (options?.logger?.debug) { - options.logger.debug(message, { - span: span!, - }); - } else { - console.error(formatted); - } - } else { - if (options?.logger?.warn) { - const params: {deprecation: boolean; span?: SourceSpan; stack?: string} = - { - deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, - }; - if (span) params.span = span; - - const stack = event.stackTrace; - if (stack) { - params.stack = options?.legacy ? removeLegacyImporter(stack) : stack; - } - - options.logger.warn(message, params); - } else { - console.error(formatted); - } - } -} - -/** - * Converts a `CompileResponse` into a `CompileResult`. - * - * Throws a `SassException` if the compilation failed. - */ -function handleCompileResponse( - response: proto.OutboundMessage_CompileResponse -): CompileResult { - if (response.result.case === 'success') { - const success = response.result.value; - const result: CompileResult = { - css: success.css, - loadedUrls: response.loadedUrls.map(url => new URL(url)), - }; - - const sourceMap = success.sourceMap; - if (sourceMap) result.sourceMap = JSON.parse(sourceMap); - return result; - } else if (response.result.case === 'failure') { - throw new Exception(response.result.value); - } else { - throw utils.compilerError('Compiler sent empty CompileResponse.'); + await compiler.dispose(); } } diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 08475ddc..c7608bb5 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -39,7 +39,7 @@ export class Compiler { private readonly stderr$ = new Subject(); /** Whether the underlying compiler has already exited. */ - private exited = false; + private disposed = false; /** Writes `buffer` to the child process's stdin. */ private writeStdin(buffer: Buffer): void { @@ -58,14 +58,14 @@ export class Compiler { return true; case 'exit': - this.exited = true; + this.disposed = true; return false; } } /** Blocks until the underlying process exits. */ private yieldUntilExit(): void { - while (!this.exited) { + while (!this.disposed) { this.yield(); } } @@ -121,14 +121,14 @@ export class Compiler { } } - private throwIfClosed(): void { - if (this.exited) { + private throwIfDisposed(): void { + if (this.disposed) { throw utils.compilerError('Sync compiler has already exited.'); } } compile(path: string, options?: Options<'sync'>): CompileResult { - this.throwIfClosed(); + this.throwIfDisposed(); const importers = new ImporterRegistry(options); return this.compileRequestSync( newCompilePathRequest(path, importers, options), @@ -138,7 +138,7 @@ export class Compiler { } compileString(source: string, options?: Options<'sync'>): CompileResult { - this.throwIfClosed(); + this.throwIfDisposed(); const importers = new ImporterRegistry(options); return this.compileRequestSync( newCompileStringRequest(source, importers, options), From 4aaa65f8da2ce96cd0c3c2ac85550cbd54c4b029 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Mon, 13 Nov 2023 23:39:21 +0000 Subject: [PATCH 04/35] Autofix --- lib/src/compiler.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index d86c3d62..f3bd0bfe 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -2,22 +2,22 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import { Observable } from 'rxjs'; +import {Observable} from 'rxjs'; import * as p from 'path'; import * as supportsColor from 'supports-color'; -import { deprotofySourceSpan } from './deprotofy-span'; -import { Dispatcher, DispatcherHandlers } from './dispatcher'; -import { Exception } from './exception'; -import { ImporterRegistry } from './importer-registry'; -import { legacyImporterProtocol } from './legacy/utils'; -import { MessageTransformer } from './message-transformer'; -import { PacketTransformer } from './packet-transformer'; +import {deprotofySourceSpan} from './deprotofy-span'; +import {Dispatcher, DispatcherHandlers} from './dispatcher'; +import {Exception} from './exception'; +import {ImporterRegistry} from './importer-registry'; +import {legacyImporterProtocol} from './legacy/utils'; +import {MessageTransformer} from './message-transformer'; +import {PacketTransformer} from './packet-transformer'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; -import { SourceSpan } from './vendor/sass'; -import { CompileResult } from './vendor/sass/compile'; -import { Options, StringOptions } from './vendor/sass/options'; +import {SourceSpan} from './vendor/sass'; +import {CompileResult} from './vendor/sass/compile'; +import {Options, StringOptions} from './vendor/sass/options'; /// Allow the legacy API to pass in an option signaling to the modern API that /// it's being run in legacy mode. @@ -145,9 +145,9 @@ export function handleLogEvent( options: OptionsWithLegacy<'sync' | 'async'> | undefined, event: proto.OutboundMessage_LogEvent ): void { - let span = event.span ? deprotofySourceSpan(event.span) : null; - let message = event.message; - let formatted = event.formatted; + const span = event.span ? deprotofySourceSpan(event.span) : null; + const message = event.message; + const formatted = event.formatted; if (event.type === proto.LogEventType.DEBUG) { if (options?.logger?.debug) { From 1caf32e6245a994691f61e2a897ec893cd3b6bf3 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Tue, 14 Nov 2023 00:03:34 +0000 Subject: [PATCH 05/35] Bring back legacy checks --- lib/src/compiler.ts | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index f3bd0bfe..ae49595c 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -10,7 +10,11 @@ import {deprotofySourceSpan} from './deprotofy-span'; import {Dispatcher, DispatcherHandlers} from './dispatcher'; import {Exception} from './exception'; import {ImporterRegistry} from './importer-registry'; -import {legacyImporterProtocol} from './legacy/utils'; +import { + legacyImporterProtocol, + removeLegacyImporter, + removeLegacyImporterFromSpan, +} from './legacy/utils'; import {MessageTransformer} from './message-transformer'; import {PacketTransformer} from './packet-transformer'; import * as utils from './utils'; @@ -145,9 +149,12 @@ export function handleLogEvent( options: OptionsWithLegacy<'sync' | 'async'> | undefined, event: proto.OutboundMessage_LogEvent ): void { - const span = event.span ? deprotofySourceSpan(event.span) : null; - const message = event.message; - const formatted = event.formatted; + let span = event.span ? deprotofySourceSpan(event.span) : null; + if (span && options?.legacy) span = removeLegacyImporterFromSpan(span); + let message = event.message; + if (options?.legacy) message = removeLegacyImporter(message); + let formatted = event.formatted; + if (options?.legacy) formatted = removeLegacyImporter(formatted); if (event.type === proto.LogEventType.DEBUG) { if (options?.logger?.debug) { @@ -159,18 +166,15 @@ export function handleLogEvent( } } else { if (options?.logger?.warn) { - const params: { - deprecation: boolean; - span?: SourceSpan; - stack?: string; - } = { - deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, - }; + const params: {deprecation: boolean; span?: SourceSpan; stack?: string} = + { + deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING, + }; if (span) params.span = span; const stack = event.stackTrace; if (stack) { - params.stack = stack; + params.stack = options?.legacy ? removeLegacyImporter(stack) : stack; } options.logger.warn(message, params); From b41959ede9763f046e37384a11479d8c46f65f74 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Tue, 14 Nov 2023 19:43:38 +0000 Subject: [PATCH 06/35] Don't autoclose the sync compiler --- lib/src/async-compiler.ts | 2 +- lib/src/sync-compiler.ts | 70 ++++++++++++++++++--------------------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index e66f1deb..19a6c34c 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -58,7 +58,7 @@ export class AsyncCompiler { private throwIfDisposed(): void { if (this.disposed) { - throw utils.compilerError('Sync compiler has already exited.'); + throw utils.compilerError('Async compiler has already been disposed.'); } } diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index c7608bb5..950519f4 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -81,49 +81,44 @@ export class Compiler { const functions = new FunctionRegistry(options?.functions); this.stderr$.subscribe(data => process.stderr.write(data)); - try { - const dispatcher = createDispatcher<'sync'>( - this.stdout$, - buffer => { - this.writeStdin(buffer); - }, - { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - } - ); - - dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); - - let error: unknown; - let response: proto.OutboundMessage_CompileResponse | undefined; - dispatcher.sendCompileRequest(request, (error_, response_) => { - if (error_) { - error = error_; - } else { - response = response_; - } - }); - - for (;;) { - if (!this.yield()) { - throw utils.compilerError('Embedded compiler exited unexpectedly.'); - } - - if (error) throw error; - if (response) return handleCompileResponse(response); + const dispatcher = createDispatcher<'sync'>( + this.stdout$, + buffer => { + this.writeStdin(buffer); + }, + { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), } - } finally { - this.dispose(); - this.yieldUntilExit(); + ); + + dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); + + let error: unknown; + let response: proto.OutboundMessage_CompileResponse | undefined; + dispatcher.sendCompileRequest(request, (error_, response_) => { + if (error_) { + error = error_; + } else { + response = response_; + } + }); + + for (;;) { + if (!this.yield()) { + throw utils.compilerError('Embedded compiler exited unexpectedly.'); + } + + if (error) throw error; + if (response) return handleCompileResponse(response); } } private throwIfDisposed(): void { if (this.disposed) { - throw utils.compilerError('Sync compiler has already exited.'); + throw utils.compilerError('Sync compiler has already been disposed.'); } } @@ -150,6 +145,7 @@ export class Compiler { /** Kills the child process, cleaning up all associated Observables. */ dispose() { this.process.stdin.end(); + this.yieldUntilExit(); } } From 7906f99c77b2d92aceab74028c74a35b92839586 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Tue, 14 Nov 2023 21:42:44 +0000 Subject: [PATCH 07/35] Update module exports --- lib/index.mjs | 10 ++++++++++ lib/index.ts | 2 ++ 2 files changed, 12 insertions(+) diff --git a/lib/index.mjs b/lib/index.mjs index ff94ed4e..10da0dd4 100644 --- a/lib/index.mjs +++ b/lib/index.mjs @@ -4,6 +4,8 @@ export const compile = sass.compile; export const compileAsync = sass.compileAsync; export const compileString = sass.compileString; export const compileStringAsync = sass.compileStringAsync; +export const initAsyncCompiler = sass.initAsyncCompiler; +export const initCompiler = sass.initCompiler; export const Logger = sass.Logger; export const CalculationInterpolation = sass.CalculationInterpolation export const CalculationOperation = sass.CalculationOperation @@ -60,6 +62,14 @@ export default { defaultExportDeprecation(); return sass.compileStringAsync; }, + get initAsyncCompiler() { + defaultExportDeprecation(); + return sass.initAsyncCompiler; + }, + get initCompiler() { + defaultExportDeprecation(); + return sass.initCompiler; + }, get Logger() { defaultExportDeprecation(); return sass.Logger; diff --git a/lib/index.ts b/lib/index.ts index 69005262..680bc72f 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -32,6 +32,8 @@ export { compileAsync, compileStringAsync, } from './src/compile'; +export {initAsyncCompiler} from './src/async-compiler'; +export {initCompiler} from './src/sync-compiler'; export {render, renderSync} from './src/legacy'; export const info = `sass-embedded\t${pkg.version}`; From ebfddd959e77c98a0a8ee347a40efb24ee2a24e2 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Tue, 14 Nov 2023 22:29:27 +0000 Subject: [PATCH 08/35] Let active compilations settle before disposing --- lib/src/async-compiler.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 19a6c34c..a082e93d 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -36,6 +36,9 @@ export class AsyncCompiler { /** Whether the underlying compiler has already exited. */ private disposed = false; + /** A record of all compilations */ + private compilations: Promise[] = []; + /** The child process's exit event. */ private readonly exit$ = new Promise(resolve => { this.process.on('exit', code => resolve(code)); @@ -108,11 +111,13 @@ export class AsyncCompiler { ): Promise { this.throwIfDisposed(); const importers = new ImporterRegistry(options); - return this.compileRequestAsync( + const compilation = this.compileRequestAsync( newCompilePathRequest(path, importers, options), importers, options ); + this.compilations.push(compilation); + return compilation; } compileStringAsync( @@ -121,16 +126,19 @@ export class AsyncCompiler { ): Promise { this.throwIfDisposed(); const importers = new ImporterRegistry(options); - return this.compileRequestAsync( + const compilation = this.compileRequestAsync( newCompileStringRequest(source, importers, options), importers, options ); + this.compilations.push(compilation); + return compilation; } /** Kills the child process, cleaning up all associated Observables. */ async dispose() { this.disposed = true; + await Promise.all(this.compilations); this.process.stdin.end(); await this.exit$; } From c47a95808c2a0a720dc8eebf29a259eeddbaaee9 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Wed, 15 Nov 2023 14:57:04 -0500 Subject: [PATCH 09/35] update copyright --- lib/src/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index ae49595c..2e485d35 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -1,4 +1,4 @@ -// Copyright 2021 Google LLC. Use of this source code is governed by an +// Copyright 2023 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. From a6ca5d99f097d772649c6fe7d8af0d97fffa6804 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Wed, 15 Nov 2023 21:04:37 +0000 Subject: [PATCH 10/35] Add try/catch to sync functions as well --- lib/src/compile.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/src/compile.ts b/lib/src/compile.ts index 9a340b21..cf91eeec 100644 --- a/lib/src/compile.ts +++ b/lib/src/compile.ts @@ -12,9 +12,11 @@ export function compile( options?: OptionsWithLegacy<'sync'> ): CompileResult { const compiler = initCompiler(); - const result = compiler.compile(path, options); - compiler.dispose(); - return result; + try { + return compiler.compile(path, options); + } finally { + compiler.dispose(); + } } export function compileString( @@ -22,9 +24,11 @@ export function compileString( options?: StringOptionsWithLegacy<'sync'> ): CompileResult { const compiler = initCompiler(); - const result = compiler.compileString(source, options); - compiler.dispose(); - return result; + try { + return compiler.compileString(source, options); + } finally { + compiler.dispose(); + } } export async function compileAsync( From 8ecb0e30cb84e3550a7d789f25416fae7d6c7d05 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 17:01:03 +0000 Subject: [PATCH 11/35] Export compiler classes --- lib/index.mjs | 2 ++ lib/index.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/index.mjs b/lib/index.mjs index 10da0dd4..6f47bf7b 100644 --- a/lib/index.mjs +++ b/lib/index.mjs @@ -4,6 +4,8 @@ export const compile = sass.compile; export const compileAsync = sass.compileAsync; export const compileString = sass.compileString; export const compileStringAsync = sass.compileStringAsync; +export const AsyncCompiler = sass.AsyncCompiler; +export const Compiler = sass.Compiler; export const initAsyncCompiler = sass.initAsyncCompiler; export const initCompiler = sass.initCompiler; export const Logger = sass.Logger; diff --git a/lib/index.ts b/lib/index.ts index 680bc72f..f8b5c779 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -32,8 +32,8 @@ export { compileAsync, compileStringAsync, } from './src/compile'; -export {initAsyncCompiler} from './src/async-compiler'; -export {initCompiler} from './src/sync-compiler'; +export {initAsyncCompiler, AsyncCompiler} from './src/async-compiler'; +export {initCompiler, Compiler} from './src/sync-compiler'; export {render, renderSync} from './src/legacy'; export const info = `sass-embedded\t${pkg.version}`; From ba7c156d2b0301974676e43f6680e74e635111e7 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 18:22:32 +0000 Subject: [PATCH 12/35] Remove complete compilations from compiler state --- lib/src/async-compiler.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index a082e93d..0ecdc0c8 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -36,8 +36,8 @@ export class AsyncCompiler { /** Whether the underlying compiler has already exited. */ private disposed = false; - /** A record of all compilations */ - private compilations: Promise[] = []; + /** A list of pending compilations */ + private compilations = new Set>(); /** The child process's exit event. */ private readonly exit$ = new Promise(resolve => { @@ -59,6 +59,14 @@ export class AsyncCompiler { this.process.stdin.write(buffer); } + /** Adds a compilation to the pending set and removes it up when it's done. */ + private addCompilation(compilation: Promise): void { + this.compilations.add(compilation); + compilation + .catch(() => {}) + .finally(() => this.compilations.delete(compilation)); + } + private throwIfDisposed(): void { if (this.disposed) { throw utils.compilerError('Async compiler has already been disposed.'); @@ -116,7 +124,7 @@ export class AsyncCompiler { importers, options ); - this.compilations.push(compilation); + this.addCompilation(compilation); return compilation; } @@ -131,7 +139,7 @@ export class AsyncCompiler { importers, options ); - this.compilations.push(compilation); + this.addCompilation(compilation); return compilation; } From cdd32ca12c739cf98367fc5c5294f8f96e2ed746 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 18:30:19 +0000 Subject: [PATCH 13/35] Update comments --- lib/src/async-compiler.ts | 12 +++++++----- lib/src/sync-compiler.ts | 10 ++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 0ecdc0c8..30ae930c 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -59,7 +59,7 @@ export class AsyncCompiler { this.process.stdin.write(buffer); } - /** Adds a compilation to the pending set and removes it up when it's done. */ + /** Adds a compilation to the pending set and removes it when it's done. */ private addCompilation(compilation: Promise): void { this.compilations.add(compilation); compilation @@ -67,15 +67,18 @@ export class AsyncCompiler { .finally(() => this.compilations.delete(compilation)); } + /** Guards against using a disposed compiler. */ private throwIfDisposed(): void { if (this.disposed) { throw utils.compilerError('Async compiler has already been disposed.'); } } - // Spins up a compiler, then sends it a compile request. Returns a promise that - // resolves with the CompileResult. Throws if there were any protocol or - // compilation errors. Shuts down the compiler after compilation. + /** + * Sends a compile request to the child process and returns a Promise that + * resolves with the CompileResult. Rejects the promise if there were any + * protocol or compilation errors. + */ private async compileRequestAsync( request: proto.InboundMessage_CompileRequest, importers: ImporterRegistry<'async'>, @@ -143,7 +146,6 @@ export class AsyncCompiler { return compilation; } - /** Kills the child process, cleaning up all associated Observables. */ async dispose() { this.disposed = true; await Promise.all(this.compilations); diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 950519f4..83112fce 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -46,6 +46,7 @@ export class Compiler { this.process.stdin.write(buffer); } + /** Yields the next event from the underlying process. */ private yield(): boolean { const event = this.process.yield(); switch (event.type) { @@ -70,9 +71,10 @@ export class Compiler { } } - // Spins up a compiler, then sends it a compile request. Returns a promise that - // resolves with the CompileResult. Throws if there were any protocol or - // compilation errors. Shuts down the compiler after compilation. + /** + * Sends a compile request to the child process and returns the CompileResult. + * Throws if there were any protocol or compilation errors. + */ private compileRequestSync( request: proto.InboundMessage_CompileRequest, importers: ImporterRegistry<'sync'>, @@ -116,6 +118,7 @@ export class Compiler { } } + /** Guards against using a disposed compiler. */ private throwIfDisposed(): void { if (this.disposed) { throw utils.compilerError('Sync compiler has already been disposed.'); @@ -142,7 +145,6 @@ export class Compiler { ); } - /** Kills the child process, cleaning up all associated Observables. */ dispose() { this.process.stdin.end(); this.yieldUntilExit(); From fbbe407b79adc730bdf1e6a2be11b0b4464d79b6 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 18:52:10 +0000 Subject: [PATCH 14/35] Add unique compilation IDs --- lib/src/compiler.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index 2e485d35..15f51b7e 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -23,6 +23,9 @@ import {SourceSpan} from './vendor/sass'; import {CompileResult} from './vendor/sass/compile'; import {Options, StringOptions} from './vendor/sass/options'; +// A module-level ID for each compilation. +let compilationId = 0; + /// Allow the legacy API to pass in an option signaling to the modern API that /// it's being run in legacy mode. /// @@ -56,11 +59,7 @@ export function createDispatcher( ); return new Dispatcher( - // Since we only use one compilation per process, we can get away with - // hardcoding a compilation ID. Once we support multiple concurrent - // compilations with the same process, we'll need to ensure each one uses a - // unique ID. - 1, + compilationId += 1, messageTransformer.outboundMessages$, message => messageTransformer.writeInboundMessage(message), handlers From c35c965b709898d95c45d084832133598caa729c Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 19:09:10 +0000 Subject: [PATCH 15/35] Lint --- lib/src/compiler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index 15f51b7e..8c6bc16e 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -59,7 +59,7 @@ export function createDispatcher( ); return new Dispatcher( - compilationId += 1, + (compilationId += 1), messageTransformer.outboundMessages$, message => messageTransformer.writeInboundMessage(message), handlers From 298daab0577711bec3c03c4168be4273ceb1a5b6 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 16 Nov 2023 19:11:00 +0000 Subject: [PATCH 16/35] Add missing getter --- lib/index.mjs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/index.mjs b/lib/index.mjs index 6f47bf7b..d0d6134c 100644 --- a/lib/index.mjs +++ b/lib/index.mjs @@ -72,6 +72,14 @@ export default { defaultExportDeprecation(); return sass.initCompiler; }, + get AsyncCompiler() { + defaultExportDeprecation(); + return sass.AsyncCompiler; + }, + get Compiler() { + defaultExportDeprecation(); + return sass.Compiler; + }, get Logger() { defaultExportDeprecation(); return sass.Logger; From 06974e1f0fdded65af93135fb1ea39fcbec2c2eb Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Tue, 21 Nov 2023 01:29:56 +0000 Subject: [PATCH 17/35] Reuse package and message transformers --- lib/src/async-compiler.ts | 36 +++++++++++++++++++++++------------- lib/src/compiler.ts | 13 +------------ lib/src/sync-compiler.ts | 36 +++++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 38 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 30ae930c..ed37000e 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -21,6 +21,8 @@ import {ImporterRegistry} from './importer-registry'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; import {CompileResult} from './vendor/sass'; +import {PacketTransformer} from './packet-transformer'; +import {MessageTransformer} from './message-transformer'; /** * An asynchronous wrapper for the embedded Sass compiler @@ -36,6 +38,9 @@ export class AsyncCompiler { /** Whether the underlying compiler has already exited. */ private disposed = false; + /** Reusable message transformer for all compilations. */ + private messageTransformer: MessageTransformer; + /** A list of pending compilations */ private compilations = new Set>(); @@ -85,20 +90,13 @@ export class AsyncCompiler { options?: OptionsWithLegacy<'async'> & {legacy?: boolean} ): Promise { const functions = new FunctionRegistry(options?.functions); - this.stderr$.subscribe(data => process.stderr.write(data)); - const dispatcher = createDispatcher<'async'>( - this.stdout$, - buffer => { - this.writeStdin(buffer); - }, - { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - } - ); + const dispatcher = createDispatcher<'async'>(this.messageTransformer, { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + }); dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); @@ -116,6 +114,18 @@ export class AsyncCompiler { ); } + /** Initialize resources shared across compilations. */ + constructor() { + this.stderr$.subscribe(data => process.stderr.write(data)); + const packetTransformer = new PacketTransformer(this.stdout$, buffer => { + this.writeStdin(buffer); + }); + this.messageTransformer = new MessageTransformer( + packetTransformer.outboundProtobufs$, + packet => packetTransformer.writeInboundProtobuf(packet) + ); + } + compileAsync( path: string, options?: OptionsWithLegacy<'async'> diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index 8c6bc16e..7c538ac5 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -2,8 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import {Observable} from 'rxjs'; - import * as p from 'path'; import * as supportsColor from 'supports-color'; import {deprotofySourceSpan} from './deprotofy-span'; @@ -16,7 +14,6 @@ import { removeLegacyImporterFromSpan, } from './legacy/utils'; import {MessageTransformer} from './message-transformer'; -import {PacketTransformer} from './packet-transformer'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; import {SourceSpan} from './vendor/sass'; @@ -47,17 +44,9 @@ export type StringOptionsWithLegacy = * Creates a dispatcher that dispatches messages from the given `stdout` stream. */ export function createDispatcher( - stdout: Observable, - writeStdin: (buffer: Buffer) => void, + messageTransformer: MessageTransformer, handlers: DispatcherHandlers ): Dispatcher { - const packetTransformer = new PacketTransformer(stdout, writeStdin); - - const messageTransformer = new MessageTransformer( - packetTransformer.outboundProtobufs$, - packet => packetTransformer.writeInboundProtobuf(packet) - ); - return new Dispatcher( (compilationId += 1), messageTransformer.outboundMessages$, diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 83112fce..3807f90e 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -20,6 +20,8 @@ import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; import {CompileResult} from './vendor/sass/compile'; import {Options} from './vendor/sass/options'; +import {PacketTransformer} from './packet-transformer'; +import {MessageTransformer} from './message-transformer'; /** * A synchronous wrapper for the embedded Sass compiler @@ -41,6 +43,9 @@ export class Compiler { /** Whether the underlying compiler has already exited. */ private disposed = false; + /** Reusable message transformer for all compilations. */ + private messageTransformer: MessageTransformer; + /** Writes `buffer` to the child process's stdin. */ private writeStdin(buffer: Buffer): void { this.process.stdin.write(buffer); @@ -81,20 +86,13 @@ export class Compiler { options?: OptionsWithLegacy<'sync'> ): CompileResult { const functions = new FunctionRegistry(options?.functions); - this.stderr$.subscribe(data => process.stderr.write(data)); - const dispatcher = createDispatcher<'sync'>( - this.stdout$, - buffer => { - this.writeStdin(buffer); - }, - { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - } - ); + const dispatcher = createDispatcher<'sync'>(this.messageTransformer, { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + }); dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); @@ -125,6 +123,18 @@ export class Compiler { } } + /** Initialize resources shared across compilations. */ + constructor() { + this.stderr$.subscribe(data => process.stderr.write(data)); + const packetTransformer = new PacketTransformer(this.stdout$, buffer => { + this.writeStdin(buffer); + }); + this.messageTransformer = new MessageTransformer( + packetTransformer.outboundProtobufs$, + packet => packetTransformer.writeInboundProtobuf(packet) + ); + } + compile(path: string, options?: Options<'sync'>): CompileResult { this.throwIfDisposed(); const importers = new ImporterRegistry(options); From 7b7fbf7b01d2850a2a9353630ffcc21768719fab Mon Sep 17 00:00:00 2001 From: James Stuckey Weber Date: Fri, 15 Dec 2023 09:47:58 -0500 Subject: [PATCH 18/35] Throw errors if Compiler classes directly contructed --- lib/src/async-compiler.ts | 16 ++++++++++++++-- lib/src/sync-compiler.ts | 16 ++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index ed37000e..22050235 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -24,6 +24,13 @@ import {CompileResult} from './vendor/sass'; import {PacketTransformer} from './packet-transformer'; import {MessageTransformer} from './message-transformer'; +/** + * Flag allowing the constructor passed by `initAsyncCompiler` so we can + * differentiate and throw an error if the `AsyncCompiler` is constructed via + * `new AsyncCompiler`. + */ +const initFlag = Symbol(); + /** * An asynchronous wrapper for the embedded Sass compiler */ @@ -115,7 +122,12 @@ export class AsyncCompiler { } /** Initialize resources shared across compilations. */ - constructor() { + constructor(flag: Symbol | undefined) { + if (flag !== initFlag) { + throw utils.compilerError( + 'AsyncCompiler can not be directly constructed. Please use `sass.initAsyncCompiler()` instead.' + ); + } this.stderr$.subscribe(data => process.stderr.write(data)); const packetTransformer = new PacketTransformer(this.stdout$, buffer => { this.writeStdin(buffer); @@ -165,5 +177,5 @@ export class AsyncCompiler { } export async function initAsyncCompiler(): Promise { - return new AsyncCompiler(); + return new AsyncCompiler(initFlag); } diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 3807f90e..196fd5b6 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -23,6 +23,13 @@ import {Options} from './vendor/sass/options'; import {PacketTransformer} from './packet-transformer'; import {MessageTransformer} from './message-transformer'; +/** + * Flag allowing the constructor passed by `initCompiler` so we can + * differentiate and throw an error if the `Compiler` is constructed via `new + * Compiler`. + */ +const initFlag = Symbol(); + /** * A synchronous wrapper for the embedded Sass compiler */ @@ -124,7 +131,12 @@ export class Compiler { } /** Initialize resources shared across compilations. */ - constructor() { + constructor(flag: Symbol | undefined) { + if (flag !== initFlag) { + throw utils.compilerError( + 'Compiler can not be directly constructed. Please use `sass.initAsyncCompiler()` instead.' + ); + } this.stderr$.subscribe(data => process.stderr.write(data)); const packetTransformer = new PacketTransformer(this.stdout$, buffer => { this.writeStdin(buffer); @@ -162,5 +174,5 @@ export class Compiler { } export function initCompiler(): Compiler { - return new Compiler(); + return new Compiler(initFlag); } From 297cf6734b082823eeedf493f39971eb24d97ad6 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Sat, 23 Dec 2023 19:37:51 +0000 Subject: [PATCH 19/35] Use compiler-level compilation IDs --- lib/src/async-compiler.ts | 16 +++++--- lib/src/compiler.test.ts | 85 +++++++++++++++++++++++++++++++++++++++ lib/src/compiler.ts | 6 +-- lib/src/sync-compiler.ts | 22 +++++++--- 4 files changed, 113 insertions(+), 16 deletions(-) create mode 100644 lib/src/compiler.test.ts diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 22050235..66611c74 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -98,12 +98,16 @@ export class AsyncCompiler { ): Promise { const functions = new FunctionRegistry(options?.functions); - const dispatcher = createDispatcher<'async'>(this.messageTransformer, { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - }); + const dispatcher = createDispatcher<'async'>( + this.compilations.size + 1, + this.messageTransformer, + { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + } + ); dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts new file mode 100644 index 00000000..e36d5d1a --- /dev/null +++ b/lib/src/compiler.test.ts @@ -0,0 +1,85 @@ +// Copyright 2023 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +// import {jest} from '@jest/globals'; +import * as compilerModule from './compiler'; +import {Compiler, initCompiler} from './sync-compiler'; +import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; + +const createDispatcher = jest.spyOn(compilerModule, 'createDispatcher'); +function getIdHistory() { + return createDispatcher.mock.calls.map(([id]) => id); +} + +afterEach(() => { + createDispatcher.mockClear(); +}); + +describe('compiler', () => { + let compiler: Compiler; + const importers = [ + { + canonicalize: () => new URL('foo:bar'), + load: () => ({ + contents: compiler.compileString('').css, + syntax: 'scss' as const, + }), + }, + ]; + + beforeEach(() => { + compiler = initCompiler(); + }); + + afterEach(() => { + compiler.dispose(); + }); + + describe('compilation ID', () => { + it('resets after callback compilations complete', () => { + compiler.compileString('@import "foo"', {importers}); + compiler.compileString(''); + expect(getIdHistory()).toEqual([1, 2, 1]); + }); + + it('keeps working after failed compilations', () => { + expect(() => compiler.compileString('invalid')).toThrow(); + compiler.compileString('@import "foo"', {importers}); + expect(getIdHistory()).toEqual([1, 1, 2]); + }); + }); +}); + +describe('asyncCompiler', () => { + let asyncCompiler: AsyncCompiler; + + beforeEach(async () => { + asyncCompiler = await initAsyncCompiler(); + }); + + afterEach(async () => { + await asyncCompiler.dispose(); + }); + + describe('compilation ID', () => { + it('resets after concurrent compilations complete', async () => { + await Promise.all( + Array.from({length: 10}, () => asyncCompiler.compileStringAsync('')) + ); + await asyncCompiler.compileStringAsync(''); + expect(getIdHistory()).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1]); + }); + + it('keeps working after failed compilations', async () => { + await expect( + asyncCompiler.compileStringAsync('invalid') + ).rejects.toThrow(); + await Promise.all([ + asyncCompiler.compileStringAsync(''), + asyncCompiler.compileStringAsync(''), + ]); + expect(getIdHistory()).toEqual([1, 1, 2]); + }); + }); +}); diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index 7c538ac5..6cb68271 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -20,9 +20,6 @@ import {SourceSpan} from './vendor/sass'; import {CompileResult} from './vendor/sass/compile'; import {Options, StringOptions} from './vendor/sass/options'; -// A module-level ID for each compilation. -let compilationId = 0; - /// Allow the legacy API to pass in an option signaling to the modern API that /// it's being run in legacy mode. /// @@ -44,11 +41,12 @@ export type StringOptionsWithLegacy = * Creates a dispatcher that dispatches messages from the given `stdout` stream. */ export function createDispatcher( + compilationId: number, messageTransformer: MessageTransformer, handlers: DispatcherHandlers ): Dispatcher { return new Dispatcher( - (compilationId += 1), + compilationId, messageTransformer.outboundMessages$, message => messageTransformer.writeInboundMessage(message), handlers diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 196fd5b6..34de851d 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -22,6 +22,7 @@ import {CompileResult} from './vendor/sass/compile'; import {Options} from './vendor/sass/options'; import {PacketTransformer} from './packet-transformer'; import {MessageTransformer} from './message-transformer'; +import {Dispatcher} from './dispatcher'; /** * Flag allowing the constructor passed by `initCompiler` so we can @@ -41,6 +42,9 @@ export class Compiler { {windowsHide: true} ); + /** A list of active dispatchers */ + private dispatchers: Set> = new Set(); + /** The buffers emitted by the child process's stdout. */ private readonly stdout$ = new Subject(); @@ -94,18 +98,24 @@ export class Compiler { ): CompileResult { const functions = new FunctionRegistry(options?.functions); - const dispatcher = createDispatcher<'sync'>(this.messageTransformer, { - handleImportRequest: request => importers.import(request), - handleFileImportRequest: request => importers.fileImport(request), - handleCanonicalizeRequest: request => importers.canonicalize(request), - handleFunctionCallRequest: request => functions.call(request), - }); + const dispatcher = createDispatcher<'sync'>( + this.dispatchers.size + 1, + this.messageTransformer, + { + handleImportRequest: request => importers.import(request), + handleFileImportRequest: request => importers.fileImport(request), + handleCanonicalizeRequest: request => importers.canonicalize(request), + handleFunctionCallRequest: request => functions.call(request), + } + ); + this.dispatchers.add(dispatcher); dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); let error: unknown; let response: proto.OutboundMessage_CompileResponse | undefined; dispatcher.sendCompileRequest(request, (error_, response_) => { + this.dispatchers.delete(dispatcher); if (error_) { error = error_; } else { From b03ef756129d882b1cba6bc3b1e5cd7457f79604 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Wed, 27 Dec 2023 00:11:09 +0000 Subject: [PATCH 20/35] Fix compilation ID reset bug in AsyncCompiler and Compiler --- lib/src/async-compiler.ts | 10 ++++++++-- lib/src/compiler.test.ts | 1 - lib/src/sync-compiler.ts | 6 +++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 66611c74..d0e39b2b 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -42,6 +42,9 @@ export class AsyncCompiler { {windowsHide: true} ); + /** The next compilation ID */ + private compilationId = 1; + /** Whether the underlying compiler has already exited. */ private disposed = false; @@ -76,7 +79,10 @@ export class AsyncCompiler { this.compilations.add(compilation); compilation .catch(() => {}) - .finally(() => this.compilations.delete(compilation)); + .finally(() => { + this.compilations.delete(compilation); + if (this.compilations.size === 0) this.compilationId = 1; + }); } /** Guards against using a disposed compiler. */ @@ -99,7 +105,7 @@ export class AsyncCompiler { const functions = new FunctionRegistry(options?.functions); const dispatcher = createDispatcher<'async'>( - this.compilations.size + 1, + this.compilationId++, this.messageTransformer, { handleImportRequest: request => importers.import(request), diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index e36d5d1a..471142c1 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -2,7 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -// import {jest} from '@jest/globals'; import * as compilerModule from './compiler'; import {Compiler, initCompiler} from './sync-compiler'; import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 34de851d..afe9dfa2 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -42,6 +42,9 @@ export class Compiler { {windowsHide: true} ); + /** The next compilation ID */ + private compilationId = 1; + /** A list of active dispatchers */ private dispatchers: Set> = new Set(); @@ -99,7 +102,7 @@ export class Compiler { const functions = new FunctionRegistry(options?.functions); const dispatcher = createDispatcher<'sync'>( - this.dispatchers.size + 1, + this.compilationId++, this.messageTransformer, { handleImportRequest: request => importers.import(request), @@ -116,6 +119,7 @@ export class Compiler { let response: proto.OutboundMessage_CompileResponse | undefined; dispatcher.sendCompileRequest(request, (error_, response_) => { this.dispatchers.delete(dispatcher); + if (this.dispatchers.size === 0) this.compilationId = 1; if (error_) { error = error_; } else { From ddfb710a06da074287e7cb71d4f9a2c11fc07def Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Wed, 27 Dec 2023 00:39:44 +0000 Subject: [PATCH 21/35] Track active compilations when building the request --- lib/src/async-compiler.ts | 52 +++++++++++++++------------------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index d0e39b2b..7e7e00d8 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -45,15 +45,16 @@ export class AsyncCompiler { /** The next compilation ID */ private compilationId = 1; + /** A list of active compilations */ + private compilations: Set> = + new Set(); + /** Whether the underlying compiler has already exited. */ private disposed = false; /** Reusable message transformer for all compilations. */ private messageTransformer: MessageTransformer; - /** A list of pending compilations */ - private compilations = new Set>(); - /** The child process's exit event. */ private readonly exit$ = new Promise(resolve => { this.process.on('exit', code => resolve(code)); @@ -74,17 +75,6 @@ export class AsyncCompiler { this.process.stdin.write(buffer); } - /** Adds a compilation to the pending set and removes it when it's done. */ - private addCompilation(compilation: Promise): void { - this.compilations.add(compilation); - compilation - .catch(() => {}) - .finally(() => { - this.compilations.delete(compilation); - if (this.compilations.size === 0) this.compilationId = 1; - }); - } - /** Guards against using a disposed compiler. */ private throwIfDisposed(): void { if (this.disposed) { @@ -114,21 +104,23 @@ export class AsyncCompiler { handleFunctionCallRequest: request => functions.call(request), } ); - dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); - return handleCompileResponse( - await new Promise( - (resolve, reject) => - dispatcher.sendCompileRequest(request, (err, response) => { - if (err) { - reject(err); - } else { - resolve(response!); - } - }) - ) + const compilation = new Promise( + (resolve, reject) => + dispatcher.sendCompileRequest(request, (err, response) => { + this.compilations.delete(compilation); + if (this.compilations.size === 0) this.compilationId = 1; + if (err) { + reject(err); + } else { + resolve(response!); + } + }) ); + this.compilations.add(compilation); + + return handleCompileResponse(await compilation); } /** Initialize resources shared across compilations. */ @@ -154,13 +146,11 @@ export class AsyncCompiler { ): Promise { this.throwIfDisposed(); const importers = new ImporterRegistry(options); - const compilation = this.compileRequestAsync( + return this.compileRequestAsync( newCompilePathRequest(path, importers, options), importers, options ); - this.addCompilation(compilation); - return compilation; } compileStringAsync( @@ -169,13 +159,11 @@ export class AsyncCompiler { ): Promise { this.throwIfDisposed(); const importers = new ImporterRegistry(options); - const compilation = this.compileRequestAsync( + return this.compileRequestAsync( newCompileStringRequest(source, importers, options), importers, options ); - this.addCompilation(compilation); - return compilation; } async dispose() { From bf7cf3c68097e93ce73693a2fa418870ae4aa65b Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 15:14:42 +0000 Subject: [PATCH 22/35] Update comments and type annotations --- lib/src/async-compiler.ts | 17 ++++++++--------- lib/src/sync-compiler.ts | 14 ++++++-------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 7e7e00d8..510ff1d1 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -31,9 +31,7 @@ import {MessageTransformer} from './message-transformer'; */ const initFlag = Symbol(); -/** - * An asynchronous wrapper for the embedded Sass compiler - */ +/** An asynchronous wrapper for the embedded Sass compiler */ export class AsyncCompiler { /** The underlying process that's being wrapped. */ private readonly process = spawn( @@ -42,18 +40,19 @@ export class AsyncCompiler { {windowsHide: true} ); - /** The next compilation ID */ + /** The next compilation ID. */ private compilationId = 1; - /** A list of active compilations */ - private compilations: Set> = - new Set(); + /** A list of active compilations. */ + private readonly compilations: Set< + Promise + > = new Set(); /** Whether the underlying compiler has already exited. */ private disposed = false; /** Reusable message transformer for all compilations. */ - private messageTransformer: MessageTransformer; + private readonly messageTransformer: MessageTransformer; /** The child process's exit event. */ private readonly exit$ = new Promise(resolve => { @@ -166,7 +165,7 @@ export class AsyncCompiler { ); } - async dispose() { + async dispose(): Promise { this.disposed = true; await Promise.all(this.compilations); this.process.stdin.end(); diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index afe9dfa2..2c23489c 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -31,9 +31,7 @@ import {Dispatcher} from './dispatcher'; */ const initFlag = Symbol(); -/** - * A synchronous wrapper for the embedded Sass compiler - */ +/** A synchronous wrapper for the embedded Sass compiler */ export class Compiler { /** The underlying process that's being wrapped. */ private readonly process = new SyncProcess( @@ -42,11 +40,11 @@ export class Compiler { {windowsHide: true} ); - /** The next compilation ID */ + /** The next compilation ID. */ private compilationId = 1; - /** A list of active dispatchers */ - private dispatchers: Set> = new Set(); + /** A list of active dispatchers. */ + private readonly dispatchers: Set> = new Set(); /** The buffers emitted by the child process's stdout. */ private readonly stdout$ = new Subject(); @@ -58,7 +56,7 @@ export class Compiler { private disposed = false; /** Reusable message transformer for all compilations. */ - private messageTransformer: MessageTransformer; + private readonly messageTransformer: MessageTransformer; /** Writes `buffer` to the child process's stdin. */ private writeStdin(buffer: Buffer): void { @@ -181,7 +179,7 @@ export class Compiler { ); } - dispose() { + dispose(): void { this.process.stdin.end(); this.yieldUntilExit(); } From 71380911af161e17f3e5291b839001d162067a14 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 16:33:04 +0000 Subject: [PATCH 23/35] Unsubscribe dispatchers when they execute their callback --- lib/src/compiler.test.ts | 20 ++++++++++++++++++ lib/src/dispatcher.ts | 45 +++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index 471142c1..30d4b56d 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -35,6 +35,14 @@ describe('compiler', () => { compiler.dispose(); }); + it('calls functions independently', () => { + const [logger1, logger2] = [jest.fn(), jest.fn()]; + compiler.compileString('@debug ""', {logger: {debug: logger1}}); + compiler.compileString('@debug ""', {logger: {debug: logger2}}); + expect(logger1).toHaveBeenCalledTimes(1); + expect(logger2).toHaveBeenCalledTimes(1); + }); + describe('compilation ID', () => { it('resets after callback compilations complete', () => { compiler.compileString('@import "foo"', {importers}); @@ -61,6 +69,18 @@ describe('asyncCompiler', () => { await asyncCompiler.dispose(); }); + it('calls functions independently', async () => { + const [logger1, logger2] = [jest.fn(), jest.fn()]; + await asyncCompiler.compileStringAsync('@debug ""', { + logger: {debug: logger1}, + }); + await asyncCompiler.compileStringAsync('@debug ""', { + logger: {debug: logger2}, + }); + expect(logger1).toHaveBeenCalledTimes(1); + expect(logger2).toHaveBeenCalledTimes(1); + }); + describe('compilation ID', () => { it('resets after concurrent compilations complete', async () => { await Promise.all( diff --git a/lib/src/dispatcher.ts b/lib/src/dispatcher.ts index 08d1271a..baa09977 100644 --- a/lib/src/dispatcher.ts +++ b/lib/src/dispatcher.ts @@ -3,13 +3,19 @@ // https://opensource.org/licenses/MIT. import {Observable, Subject} from 'rxjs'; -import {filter, map, mergeMap} from 'rxjs/operators'; +import {filter, map, mergeMap, takeUntil} from 'rxjs/operators'; import {OutboundResponse} from './messages'; import * as proto from './vendor/embedded_sass_pb'; import {RequestTracker} from './request-tracker'; import {PromiseOr, compilerError, thenOr, hostError} from './utils'; +// A callback that accepts a response or error. +type ResponseCallback = ( + err: unknown, + response: proto.OutboundMessage_CompileResponse | undefined +) => void; + /** * Dispatches requests, responses, and events for a single compilation. * @@ -50,6 +56,11 @@ export class Dispatcher { */ readonly error$ = this.errorInternal$.pipe(); + // Subject to unsubscribe from all outbound messages to prevent dispatchers + // that happen to share a compilation ID from receiving messages intended for + // past dispatchers. + private readonly unsubscribe$ = new Subject(); + /** * Outbound log events. If an error occurs, the dispatcher closes this * silently. @@ -82,7 +93,8 @@ export class Dispatcher { return result instanceof Promise ? result.then(() => message) : [message]; - }) + }), + takeUntil(this.unsubscribe$) ) .subscribe({ next: message => this.messages$.next(message), @@ -94,9 +106,16 @@ export class Dispatcher { }); } + /** Stop the outbound message subscription. */ + unsubscribe(): void { + this.unsubscribe$.next(undefined); + this.unsubscribe$.complete(); + } + /** * Sends a CompileRequest inbound. Passes the corresponding outbound - * CompileResponse or an error to `callback`. + * CompileResponse or an error to `callback` and unsubscribes from all + * outbound events. * * This uses an old-style callback argument so that it can work either * synchronously or asynchronously. If the underlying stdout stream emits @@ -104,13 +123,16 @@ export class Dispatcher { */ sendCompileRequest( request: proto.InboundMessage_CompileRequest, - callback: ( - err: unknown, - response: proto.OutboundMessage_CompileResponse | undefined - ) => void + callback: ResponseCallback ): void { + // Call the callback but unsubscribe first + const callback_: ResponseCallback = (err, response) => { + this.unsubscribe(); + return callback(err, response); + }; + if (this.messages$.isStopped) { - callback(new Error('Tried writing to closed dispatcher'), undefined); + callback_(new Error('Tried writing to closed dispatcher'), undefined); return; } @@ -119,9 +141,11 @@ export class Dispatcher { filter(message => message.message.case === 'compileResponse'), map(message => message.message.value as OutboundResponse) ) - .subscribe({next: response => callback(null, response)}); + .subscribe({next: response => callback_(null, response)}); - this.error$.subscribe({error: error => callback(error, undefined)}); + this.error$.subscribe({ + error: error => callback_(error, undefined), + }); try { this.writeInboundMessage([ @@ -140,6 +164,7 @@ export class Dispatcher { private throwAndClose(error: unknown): void { this.messages$.complete(); this.errorInternal$.error(error); + this.unsubscribe(); } // Keeps track of all outbound messages. If the outbound `message` contains a From d79ada443bc3edcaf9341de16304595e326fbe90 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 17:56:58 +0000 Subject: [PATCH 24/35] Use consistent cwd for compiler processes --- lib/src/async-compiler.ts | 7 ++++--- lib/src/compiler.test.ts | 32 ++++++++++++++++++++++++++++++-- lib/src/sync-compiler.ts | 9 +++++---- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/lib/src/async-compiler.ts b/lib/src/async-compiler.ts index 510ff1d1..49ad26a2 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/async-compiler.ts @@ -6,6 +6,7 @@ import {spawn} from 'child_process'; import {Observable} from 'rxjs'; import {takeUntil} from 'rxjs/operators'; +import * as path from 'path'; import { OptionsWithLegacy, StringOptionsWithLegacy, @@ -18,11 +19,11 @@ import { import {compilerCommand} from './compiler-path'; import {FunctionRegistry} from './function-registry'; import {ImporterRegistry} from './importer-registry'; +import {MessageTransformer} from './message-transformer'; +import {PacketTransformer} from './packet-transformer'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; import {CompileResult} from './vendor/sass'; -import {PacketTransformer} from './packet-transformer'; -import {MessageTransformer} from './message-transformer'; /** * Flag allowing the constructor passed by `initAsyncCompiler` so we can @@ -37,7 +38,7 @@ export class AsyncCompiler { private readonly process = spawn( compilerCommand[0], [...compilerCommand.slice(1), '--embedded'], - {windowsHide: true} + {cwd: path.dirname(compilerCommand[0]), windowsHide: true} ); /** The next compilation ID. */ diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index 30d4b56d..0fe38dc5 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -1,10 +1,12 @@ // Copyright 2023 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. - +import * as fs from 'fs'; +import * as path from 'path'; +import {chdir} from 'process'; +import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; import * as compilerModule from './compiler'; import {Compiler, initCompiler} from './sync-compiler'; -import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; const createDispatcher = jest.spyOn(compilerModule, 'createDispatcher'); function getIdHistory() { @@ -43,6 +45,18 @@ describe('compiler', () => { expect(logger2).toHaveBeenCalledTimes(1); }); + it('survives the removal of the working directory', () => { + const oldDir = fs.mkdtempSync('sass-spec-'); + chdir(oldDir); + const tmpCompiler = initCompiler(); + chdir('..'); + fs.rmSync(oldDir, {recursive: true}); + fs.writeFileSync('foo.scss', 'a {b: c}'); + expect(() => tmpCompiler.compile(path.resolve('foo.scss'))).not.toThrow(); + tmpCompiler.dispose(); + fs.rmSync('foo.scss'); + }); + describe('compilation ID', () => { it('resets after callback compilations complete', () => { compiler.compileString('@import "foo"', {importers}); @@ -69,6 +83,20 @@ describe('asyncCompiler', () => { await asyncCompiler.dispose(); }); + it('survives the removal of the working directory', async () => { + const oldDir = fs.mkdtempSync('sass-spec-'); + chdir(oldDir); + const tmpCompiler = await initAsyncCompiler(); + chdir('..'); + fs.rmSync(oldDir, {recursive: true}); + fs.writeFileSync('foo.scss', 'a {b: c}'); + expect( + tmpCompiler.compileAsync(path.resolve('foo.scss')) + ).resolves.not.toThrow(); + await tmpCompiler.dispose(); + fs.rmSync('foo.scss'); + }); + it('calls functions independently', async () => { const [logger1, logger2] = [jest.fn(), jest.fn()]; await asyncCompiler.compileStringAsync('@debug ""', { diff --git a/lib/src/sync-compiler.ts b/lib/src/sync-compiler.ts index 2c23489c..96efe7ce 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/sync-compiler.ts @@ -4,6 +4,7 @@ import {Subject} from 'rxjs'; +import * as path from 'path'; import { OptionsWithLegacy, createDispatcher, @@ -13,16 +14,16 @@ import { newCompileStringRequest, } from './compiler'; import {compilerCommand} from './compiler-path'; +import {Dispatcher} from './dispatcher'; import {FunctionRegistry} from './function-registry'; import {ImporterRegistry} from './importer-registry'; +import {MessageTransformer} from './message-transformer'; +import {PacketTransformer} from './packet-transformer'; import {SyncProcess} from './sync-process'; import * as utils from './utils'; import * as proto from './vendor/embedded_sass_pb'; import {CompileResult} from './vendor/sass/compile'; import {Options} from './vendor/sass/options'; -import {PacketTransformer} from './packet-transformer'; -import {MessageTransformer} from './message-transformer'; -import {Dispatcher} from './dispatcher'; /** * Flag allowing the constructor passed by `initCompiler` so we can @@ -37,7 +38,7 @@ export class Compiler { private readonly process = new SyncProcess( compilerCommand[0], [...compilerCommand.slice(1), '--embedded'], - {windowsHide: true} + {cwd: path.dirname(compilerCommand[0]), windowsHide: true} ); /** The next compilation ID. */ From 26a03385a3a1e20430f339ed929b0fed30b02481 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 22:44:27 +0000 Subject: [PATCH 25/35] Resolve paths before sending them to the compiler process --- lib/src/compiler.test.ts | 10 +++++----- lib/src/compiler.ts | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index 0fe38dc5..e5bd4faf 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -1,8 +1,8 @@ // Copyright 2023 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. + import * as fs from 'fs'; -import * as path from 'path'; import {chdir} from 'process'; import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; import * as compilerModule from './compiler'; @@ -45,14 +45,14 @@ describe('compiler', () => { expect(logger2).toHaveBeenCalledTimes(1); }); - it('survives the removal of the working directory', () => { + it('handles the removal of the working directory', () => { const oldDir = fs.mkdtempSync('sass-spec-'); chdir(oldDir); const tmpCompiler = initCompiler(); chdir('..'); fs.rmSync(oldDir, {recursive: true}); fs.writeFileSync('foo.scss', 'a {b: c}'); - expect(() => tmpCompiler.compile(path.resolve('foo.scss'))).not.toThrow(); + expect(() => tmpCompiler.compile('foo.scss')).not.toThrow(); tmpCompiler.dispose(); fs.rmSync('foo.scss'); }); @@ -83,7 +83,7 @@ describe('asyncCompiler', () => { await asyncCompiler.dispose(); }); - it('survives the removal of the working directory', async () => { + it('handles the removal of the working directory', async () => { const oldDir = fs.mkdtempSync('sass-spec-'); chdir(oldDir); const tmpCompiler = await initAsyncCompiler(); @@ -91,7 +91,7 @@ describe('asyncCompiler', () => { fs.rmSync(oldDir, {recursive: true}); fs.writeFileSync('foo.scss', 'a {b: c}'); expect( - tmpCompiler.compileAsync(path.resolve('foo.scss')) + tmpCompiler.compileAsync('foo.scss') ).resolves.not.toThrow(); await tmpCompiler.dispose(); fs.rmSync('foo.scss'); diff --git a/lib/src/compiler.ts b/lib/src/compiler.ts index 6cb68271..7dd99cd0 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler.ts @@ -93,8 +93,9 @@ export function newCompilePathRequest( importers: ImporterRegistry<'sync' | 'async'>, options?: Options<'sync' | 'async'> ): proto.InboundMessage_CompileRequest { + const absPath = p.resolve(path); const request = newCompileRequest(importers, options); - request.input = {case: 'path', value: path}; + request.input = {case: 'path', value: absPath}; return request; } From 55e64f2ad32ffd0007f2ae0633c8692fb5fd1f47 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 22:46:25 +0000 Subject: [PATCH 26/35] Lint --- lib/src/compiler.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index e5bd4faf..bf75f20c 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -90,9 +90,7 @@ describe('asyncCompiler', () => { chdir('..'); fs.rmSync(oldDir, {recursive: true}); fs.writeFileSync('foo.scss', 'a {b: c}'); - expect( - tmpCompiler.compileAsync('foo.scss') - ).resolves.not.toThrow(); + expect(tmpCompiler.compileAsync('foo.scss')).resolves.not.toThrow(); await tmpCompiler.dispose(); fs.rmSync('foo.scss'); }); From 612db591eecdaa9d873d4c88c660fb2b49b68e95 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 22:50:45 +0000 Subject: [PATCH 27/35] Add missing await --- lib/src/compiler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index bf75f20c..42cc0039 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -90,7 +90,7 @@ describe('asyncCompiler', () => { chdir('..'); fs.rmSync(oldDir, {recursive: true}); fs.writeFileSync('foo.scss', 'a {b: c}'); - expect(tmpCompiler.compileAsync('foo.scss')).resolves.not.toThrow(); + await expect(tmpCompiler.compileAsync('foo.scss')).resolves.not.toThrow(); await tmpCompiler.dispose(); fs.rmSync('foo.scss'); }); From 329fa945783920e9cee059a5de201c1a405c4d28 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 4 Jan 2024 23:17:55 +0000 Subject: [PATCH 28/35] Clean up Dispatcher class --- lib/src/dispatcher.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/src/dispatcher.ts b/lib/src/dispatcher.ts index baa09977..0be953d4 100644 --- a/lib/src/dispatcher.ts +++ b/lib/src/dispatcher.ts @@ -45,6 +45,11 @@ export class Dispatcher { // dispatching messages, this completes. private readonly messages$ = new Subject(); + // Subject to unsubscribe from all outbound messages to prevent dispatchers + // that happen to share a compilation ID from receiving messages intended for + // past dispatchers. + private readonly unsubscribe$ = new Subject(); + // If the dispatcher encounters an error, this errors out. It is publicly // exposed as a readonly Observable. private readonly errorInternal$ = new Subject(); @@ -56,11 +61,6 @@ export class Dispatcher { */ readonly error$ = this.errorInternal$.pipe(); - // Subject to unsubscribe from all outbound messages to prevent dispatchers - // that happen to share a compilation ID from receiving messages intended for - // past dispatchers. - private readonly unsubscribe$ = new Subject(); - /** * Outbound log events. If an error occurs, the dispatcher closes this * silently. @@ -106,12 +106,6 @@ export class Dispatcher { }); } - /** Stop the outbound message subscription. */ - unsubscribe(): void { - this.unsubscribe$.next(undefined); - this.unsubscribe$.complete(); - } - /** * Sends a CompileRequest inbound. Passes the corresponding outbound * CompileResponse or an error to `callback` and unsubscribes from all @@ -159,6 +153,12 @@ export class Dispatcher { } } + // Stop the outbound message subscription. + private unsubscribe(): void { + this.unsubscribe$.next(undefined); + this.unsubscribe$.complete(); + } + // Rejects with `error` all promises awaiting an outbound response, and // silently closes all subscriptions awaiting outbound events. private throwAndClose(error: unknown): void { From 25565259d3cc5acf1cf803b5a41eae315ca42ead Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Sat, 6 Jan 2024 01:49:02 +0000 Subject: [PATCH 29/35] Update compiler imports and file structure --- lib/index.ts | 4 ++-- lib/src/compile.ts | 6 ++--- lib/src/compiler.test.ts | 6 ++--- .../{async-compiler.ts => compiler/async.ts} | 18 +++++++------- .../{sync-compiler.ts => compiler/sync.ts} | 24 +++++++++---------- lib/src/{compiler.ts => compiler/utils.ts} | 22 ++++++++--------- 6 files changed, 40 insertions(+), 40 deletions(-) rename lib/src/{async-compiler.ts => compiler/async.ts} (92%) rename lib/src/{sync-compiler.ts => compiler/sync.ts} (90%) rename lib/src/{compiler.ts => compiler/utils.ts} (91%) diff --git a/lib/index.ts b/lib/index.ts index f8b5c779..89d715f4 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -32,8 +32,8 @@ export { compileAsync, compileStringAsync, } from './src/compile'; -export {initAsyncCompiler, AsyncCompiler} from './src/async-compiler'; -export {initCompiler, Compiler} from './src/sync-compiler'; +export {initAsyncCompiler, AsyncCompiler} from './src/compiler/async'; +export {initCompiler, Compiler} from './src/compiler/sync'; export {render, renderSync} from './src/legacy'; export const info = `sass-embedded\t${pkg.version}`; diff --git a/lib/src/compile.ts b/lib/src/compile.ts index cf91eeec..c6b4f5a2 100644 --- a/lib/src/compile.ts +++ b/lib/src/compile.ts @@ -2,9 +2,9 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import {initAsyncCompiler} from './async-compiler'; -import {OptionsWithLegacy, StringOptionsWithLegacy} from './compiler'; -import {initCompiler} from './sync-compiler'; +import {initAsyncCompiler} from './compiler/async'; +import {OptionsWithLegacy, StringOptionsWithLegacy} from './compiler/utils'; +import {initCompiler} from './compiler/sync'; import {CompileResult} from './vendor/sass'; export function compile( diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index 42cc0039..12ec5d5b 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -4,9 +4,9 @@ import * as fs from 'fs'; import {chdir} from 'process'; -import {AsyncCompiler, initAsyncCompiler} from './async-compiler'; -import * as compilerModule from './compiler'; -import {Compiler, initCompiler} from './sync-compiler'; +import {AsyncCompiler, initAsyncCompiler} from './compiler/async'; +import * as compilerModule from './compiler/utils'; +import {Compiler, initCompiler} from './compiler/sync'; const createDispatcher = jest.spyOn(compilerModule, 'createDispatcher'); function getIdHistory() { diff --git a/lib/src/async-compiler.ts b/lib/src/compiler/async.ts similarity index 92% rename from lib/src/async-compiler.ts rename to lib/src/compiler/async.ts index 49ad26a2..f79ee8ea 100644 --- a/lib/src/async-compiler.ts +++ b/lib/src/compiler/async.ts @@ -15,15 +15,15 @@ import { handleLogEvent, newCompilePathRequest, newCompileStringRequest, -} from './compiler'; -import {compilerCommand} from './compiler-path'; -import {FunctionRegistry} from './function-registry'; -import {ImporterRegistry} from './importer-registry'; -import {MessageTransformer} from './message-transformer'; -import {PacketTransformer} from './packet-transformer'; -import * as utils from './utils'; -import * as proto from './vendor/embedded_sass_pb'; -import {CompileResult} from './vendor/sass'; +} from './utils'; +import {compilerCommand} from '../compiler-path'; +import {FunctionRegistry} from '../function-registry'; +import {ImporterRegistry} from '../importer-registry'; +import {MessageTransformer} from '../message-transformer'; +import {PacketTransformer} from '../packet-transformer'; +import * as utils from '../utils'; +import * as proto from '../vendor/embedded_sass_pb'; +import {CompileResult} from '../vendor/sass'; /** * Flag allowing the constructor passed by `initAsyncCompiler` so we can diff --git a/lib/src/sync-compiler.ts b/lib/src/compiler/sync.ts similarity index 90% rename from lib/src/sync-compiler.ts rename to lib/src/compiler/sync.ts index 96efe7ce..5888b546 100644 --- a/lib/src/sync-compiler.ts +++ b/lib/src/compiler/sync.ts @@ -12,18 +12,18 @@ import { handleLogEvent, newCompilePathRequest, newCompileStringRequest, -} from './compiler'; -import {compilerCommand} from './compiler-path'; -import {Dispatcher} from './dispatcher'; -import {FunctionRegistry} from './function-registry'; -import {ImporterRegistry} from './importer-registry'; -import {MessageTransformer} from './message-transformer'; -import {PacketTransformer} from './packet-transformer'; -import {SyncProcess} from './sync-process'; -import * as utils from './utils'; -import * as proto from './vendor/embedded_sass_pb'; -import {CompileResult} from './vendor/sass/compile'; -import {Options} from './vendor/sass/options'; +} from './utils'; +import {compilerCommand} from '../compiler-path'; +import {Dispatcher} from '../dispatcher'; +import {FunctionRegistry} from '../function-registry'; +import {ImporterRegistry} from '../importer-registry'; +import {MessageTransformer} from '../message-transformer'; +import {PacketTransformer} from '../packet-transformer'; +import {SyncProcess} from '../sync-process'; +import * as utils from '../utils'; +import * as proto from '../vendor/embedded_sass_pb'; +import {CompileResult} from '../vendor/sass/compile'; +import {Options} from '../vendor/sass/options'; /** * Flag allowing the constructor passed by `initCompiler` so we can diff --git a/lib/src/compiler.ts b/lib/src/compiler/utils.ts similarity index 91% rename from lib/src/compiler.ts rename to lib/src/compiler/utils.ts index 7dd99cd0..d3b48cc4 100644 --- a/lib/src/compiler.ts +++ b/lib/src/compiler/utils.ts @@ -4,21 +4,21 @@ import * as p from 'path'; import * as supportsColor from 'supports-color'; -import {deprotofySourceSpan} from './deprotofy-span'; -import {Dispatcher, DispatcherHandlers} from './dispatcher'; -import {Exception} from './exception'; -import {ImporterRegistry} from './importer-registry'; +import {deprotofySourceSpan} from '../deprotofy-span'; +import {Dispatcher, DispatcherHandlers} from '../dispatcher'; +import {Exception} from '../exception'; +import {ImporterRegistry} from '../importer-registry'; import { legacyImporterProtocol, removeLegacyImporter, removeLegacyImporterFromSpan, -} from './legacy/utils'; -import {MessageTransformer} from './message-transformer'; -import * as utils from './utils'; -import * as proto from './vendor/embedded_sass_pb'; -import {SourceSpan} from './vendor/sass'; -import {CompileResult} from './vendor/sass/compile'; -import {Options, StringOptions} from './vendor/sass/options'; +} from '../legacy/utils'; +import {MessageTransformer} from '../message-transformer'; +import * as utils from '../utils'; +import * as proto from '../vendor/embedded_sass_pb'; +import {SourceSpan} from '../vendor/sass'; +import {CompileResult} from '../vendor/sass/compile'; +import {Options, StringOptions} from '../vendor/sass/options'; /// Allow the legacy API to pass in an option signaling to the modern API that /// it's being run in legacy mode. From f5c2e8ad5fe0dd2ae5017ff6d01150e1c34c8a01 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Sat, 6 Jan 2024 01:50:40 +0000 Subject: [PATCH 30/35] Update copyright year --- lib/src/compiler.test.ts | 2 +- lib/src/compiler/async.ts | 2 +- lib/src/compiler/sync.ts | 2 +- lib/src/compiler/utils.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index 12ec5d5b..f99190de 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -1,4 +1,4 @@ -// Copyright 2023 Google Inc. Use of this source code is governed by an +// Copyright 2024 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/compiler/async.ts b/lib/src/compiler/async.ts index f79ee8ea..c2bf2082 100644 --- a/lib/src/compiler/async.ts +++ b/lib/src/compiler/async.ts @@ -1,4 +1,4 @@ -// Copyright 2020 Google Inc. Use of this source code is governed by an +// Copyright 2024 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/compiler/sync.ts b/lib/src/compiler/sync.ts index 5888b546..0596af52 100644 --- a/lib/src/compiler/sync.ts +++ b/lib/src/compiler/sync.ts @@ -1,4 +1,4 @@ -// Copyright 2021 Google LLC. Use of this source code is governed by an +// Copyright 2024 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/compiler/utils.ts b/lib/src/compiler/utils.ts index d3b48cc4..3e070b33 100644 --- a/lib/src/compiler/utils.ts +++ b/lib/src/compiler/utils.ts @@ -1,4 +1,4 @@ -// Copyright 2023 Google LLC. Use of this source code is governed by an +// Copyright 2024 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. From 66a7f66d0f2454a92a3866d872c529bbc682b9b7 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Sat, 6 Jan 2024 02:05:38 +0000 Subject: [PATCH 31/35] Add comments --- lib/src/compiler/async.ts | 14 ++++++++++++-- lib/src/compiler/sync.ts | 14 ++++++++++++-- lib/src/dispatcher.ts | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/src/compiler/async.ts b/lib/src/compiler/async.ts index c2bf2082..1ae5d86f 100644 --- a/lib/src/compiler/async.ts +++ b/lib/src/compiler/async.ts @@ -38,7 +38,13 @@ export class AsyncCompiler { private readonly process = spawn( compilerCommand[0], [...compilerCommand.slice(1), '--embedded'], - {cwd: path.dirname(compilerCommand[0]), windowsHide: true} + { + // Use the command's cwd so the compiler survives the removal of the + // current working directory. + // https://github.com/sass/embedded-host-node/pull/261#discussion_r1438712923 + cwd: path.dirname(compilerCommand[0]), + windowsHide: true, + } ); /** The next compilation ID. */ @@ -110,6 +116,9 @@ export class AsyncCompiler { (resolve, reject) => dispatcher.sendCompileRequest(request, (err, response) => { this.compilations.delete(compilation); + // Reset the compilation ID when the compiler goes idle (no active + // compilations) to avoid overflowing it. + // https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794 if (this.compilations.size === 0) this.compilationId = 1; if (err) { reject(err); @@ -127,7 +136,8 @@ export class AsyncCompiler { constructor(flag: Symbol | undefined) { if (flag !== initFlag) { throw utils.compilerError( - 'AsyncCompiler can not be directly constructed. Please use `sass.initAsyncCompiler()` instead.' + 'AsyncCompiler can not be directly constructed. ' + + 'Please use `sass.initAsyncCompiler()` instead.' ); } this.stderr$.subscribe(data => process.stderr.write(data)); diff --git a/lib/src/compiler/sync.ts b/lib/src/compiler/sync.ts index 0596af52..50b7bb33 100644 --- a/lib/src/compiler/sync.ts +++ b/lib/src/compiler/sync.ts @@ -38,7 +38,13 @@ export class Compiler { private readonly process = new SyncProcess( compilerCommand[0], [...compilerCommand.slice(1), '--embedded'], - {cwd: path.dirname(compilerCommand[0]), windowsHide: true} + { + // Use the command's cwd so the compiler survives the removal of the + // current working directory. + // https://github.com/sass/embedded-host-node/pull/261#discussion_r1438712923 + cwd: path.dirname(compilerCommand[0]), + windowsHide: true, + } ); /** The next compilation ID. */ @@ -118,6 +124,9 @@ export class Compiler { let response: proto.OutboundMessage_CompileResponse | undefined; dispatcher.sendCompileRequest(request, (error_, response_) => { this.dispatchers.delete(dispatcher); + // Reset the compilation ID when the compiler goes idle (no active + // dispatchers) to avoid overflowing it. + // https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794 if (this.dispatchers.size === 0) this.compilationId = 1; if (error_) { error = error_; @@ -147,7 +156,8 @@ export class Compiler { constructor(flag: Symbol | undefined) { if (flag !== initFlag) { throw utils.compilerError( - 'Compiler can not be directly constructed. Please use `sass.initAsyncCompiler()` instead.' + 'Compiler can not be directly constructed. ' + + 'Please use `sass.initAsyncCompiler()` instead.' ); } this.stderr$.subscribe(data => process.stderr.write(data)); diff --git a/lib/src/dispatcher.ts b/lib/src/dispatcher.ts index 0be953d4..493ec3d7 100644 --- a/lib/src/dispatcher.ts +++ b/lib/src/dispatcher.ts @@ -46,7 +46,7 @@ export class Dispatcher { private readonly messages$ = new Subject(); // Subject to unsubscribe from all outbound messages to prevent dispatchers - // that happen to share a compilation ID from receiving messages intended for + // that happen to reuse a compilation ID from receiving messages intended for // past dispatchers. private readonly unsubscribe$ = new Subject(); From 55a55edfb8033147eb27fd25519a6e38b81878f8 Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Mon, 8 Jan 2024 11:28:26 -0600 Subject: [PATCH 32/35] Update comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: なつき --- lib/src/dispatcher.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/dispatcher.ts b/lib/src/dispatcher.ts index 493ec3d7..a99dee96 100644 --- a/lib/src/dispatcher.ts +++ b/lib/src/dispatcher.ts @@ -45,9 +45,9 @@ export class Dispatcher { // dispatching messages, this completes. private readonly messages$ = new Subject(); - // Subject to unsubscribe from all outbound messages to prevent dispatchers - // that happen to reuse a compilation ID from receiving messages intended for - // past dispatchers. + // Subject to unsubscribe from all outbound messages to prevent past + // dispatchers with compilation IDs reused by future dispatchers from + // receiving messages intended for future dispatchers. private readonly unsubscribe$ = new Subject(); // If the dispatcher encounters an error, this errors out. It is publicly From 2b24ec44d47115b53c6f31824ba5d14deafda9e8 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Mon, 8 Jan 2024 14:44:17 -0500 Subject: [PATCH 33/35] update comments --- lib/src/compiler/utils.ts | 24 ++++++++++++++---------- lib/src/legacy/utils.ts | 20 ++++++++++---------- tool/utils.ts | 4 ++-- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/src/compiler/utils.ts b/lib/src/compiler/utils.ts index 3e070b33..4ef6648a 100644 --- a/lib/src/compiler/utils.ts +++ b/lib/src/compiler/utils.ts @@ -20,20 +20,24 @@ import {SourceSpan} from '../vendor/sass'; import {CompileResult} from '../vendor/sass/compile'; import {Options, StringOptions} from '../vendor/sass/options'; -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. +/** + * Allow the legacy API to pass in an option signaling to the modern API that + * it's being run in legacy mode. + * + * This is not intended for API users to pass in, and may be broken without + * warning in the future. + */ export type OptionsWithLegacy = Options & { legacy?: boolean; }; -/// Allow the legacy API to pass in an option signaling to the modern API that -/// it's being run in legacy mode. -/// -/// This is not intended for API users to pass in, and may be broken without -/// warning in the future. +/** + * Allow the legacy API to pass in an option signaling to the modern API that + * it's being run in legacy mode. + * + * This is not intended for API users to pass in, and may be broken without + * warning in the future. + */ export type StringOptionsWithLegacy = StringOptions & {legacy?: boolean}; diff --git a/lib/src/legacy/utils.ts b/lib/src/legacy/utils.ts index 05ffe42b..7a72472f 100644 --- a/lib/src/legacy/utils.ts +++ b/lib/src/legacy/utils.ts @@ -22,34 +22,34 @@ export const legacyImporterProtocol = 'legacy-importer:'; */ export const legacyImporterProtocolPrefix = 'legacy-importer-'; -/// A regular expression that matches legacy importer protocol syntax that -/// should be removed from human-readable messages. +// A regular expression that matches legacy importer protocol syntax that +// should be removed from human-readable messages. const removeLegacyImporterRegExp = new RegExp( `${legacyImporterProtocol}|${legacyImporterProtocolPrefix}`, 'g' ); -/// Returns `string` with all instances of legacy importer syntax removed. +// Returns `string` with all instances of legacy importer syntax removed. export function removeLegacyImporter(string: string): string { return string.replace(removeLegacyImporterRegExp, ''); } -/// Returns a copy of [span] with the URL updated to remove legacy importer -/// syntax. +// Returns a copy of [span] with the URL updated to remove legacy importer +// syntax. export function removeLegacyImporterFromSpan(span: SourceSpan): SourceSpan { if (!span.url) return span; return {...span, url: new URL(removeLegacyImporter(span.url.toString()))}; } -/// Converts [path] to a `file:` URL and adds the [legacyImporterProtocolPrefix] -/// to the beginning so we can distinguish it from manually-specified absolute -/// `file:` URLs. +// Converts [path] to a `file:` URL and adds the [legacyImporterProtocolPrefix] +// to the beginning so we can distinguish it from manually-specified absolute +// `file:` URLs. export function pathToLegacyFileUrl(path: string): URL { return new URL(`${legacyImporterProtocolPrefix}${pathToFileURL(path)}`); } -/// Converts a `file:` URL with [legacyImporterProtocolPrefix] to the filesystem -/// path which it represents. +// Converts a `file:` URL with [legacyImporterProtocolPrefix] to the filesystem +// path which it represents. export function legacyFileUrlToPath(url: URL): string { assert.equal(url.protocol, legacyImporterFileProtocol); const originalUrl = url diff --git a/tool/utils.ts b/tool/utils.ts index e0cc581e..fe1ee82a 100644 --- a/tool/utils.ts +++ b/tool/utils.ts @@ -72,7 +72,7 @@ export async function cleanDir(dir: string): Promise { } } -/// Returns whether [path1] and [path2] are symlinks that refer to the same file. +// Returns whether [path1] and [path2] are symlinks that refer to the same file. export async function sameTarget( path1: string, path2: string @@ -83,7 +83,7 @@ export async function sameTarget( return realpath1 === (await tryRealpath(path2)); } -/// Like `fs.realpath()`, but returns `null` if the path doesn't exist on disk. +// Like `fs.realpath()`, but returns `null` if the path doesn't exist on disk. async function tryRealpath(path: string): Promise { try { return await fs.realpath(p.resolve(path)); From a222ff21a0d4b1060e8e901f3855e01ed0d8a828 Mon Sep 17 00:00:00 2001 From: Jonny Gerig Meyer Date: Mon, 8 Jan 2024 14:47:48 -0500 Subject: [PATCH 34/35] copyrights are inconsistent in this repo --- lib/src/compiler.test.ts | 2 +- lib/src/compiler/async.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/compiler.test.ts b/lib/src/compiler.test.ts index f99190de..58fbc61f 100644 --- a/lib/src/compiler.test.ts +++ b/lib/src/compiler.test.ts @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2024 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/compiler/async.ts b/lib/src/compiler/async.ts index 1ae5d86f..6b4d77a4 100644 --- a/lib/src/compiler/async.ts +++ b/lib/src/compiler/async.ts @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2024 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. From 84ae6a41174c52e11df5dd72482fba2367b5639a Mon Sep 17 00:00:00 2001 From: Ed Rivas Date: Thu, 11 Jan 2024 22:58:21 +0000 Subject: [PATCH 35/35] Add private id to FunctionRegistry --- lib/src/function-registry.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/src/function-registry.ts b/lib/src/function-registry.ts index d82acc4a..0c4a3b81 100644 --- a/lib/src/function-registry.ts +++ b/lib/src/function-registry.ts @@ -12,12 +12,6 @@ import {PromiseOr, catchOr, compilerError, thenOr} from './utils'; import {Protofier} from './protofier'; import {Value} from './value'; -/** - * The next ID to use for a function. The embedded protocol requires that - * function IDs be globally unique. - */ -let nextFunctionID = 0; - /** * Tracks functions that are defined on the host so that the compiler can * execute them. @@ -27,6 +21,9 @@ export class FunctionRegistry { private readonly functionsById = new Map>(); private readonly idsByFunction = new Map, number>(); + /** The next ID to use for a function. */ + private id = 0; + constructor(functionsBySignature?: Record>) { for (const [signature, fn] of Object.entries(functionsBySignature ?? {})) { const openParen = signature.indexOf('('); @@ -41,8 +38,8 @@ export class FunctionRegistry { /** Registers `fn` as a function that can be called using the returned ID. */ register(fn: CustomFunction): number { return utils.putIfAbsent(this.idsByFunction, fn, () => { - const id = nextFunctionID; - nextFunctionID += 1; + const id = this.id; + this.id += 1; this.functionsById.set(id, fn); return id; });