Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Merge branch 'main' of github.com:sass/embedded-host-node into feature.color-4 #256

Merged
merged 11 commits into from
Oct 6, 2023
72 changes: 72 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,75 @@
## 1.68.0

* Fix the source spans associated with the `abs-percent` deprecation.

### JS API

* Non-filesystem importers can now set the `nonCanonicalScheme` field, which
declares that one or more URL schemes (without `:`) will never be used for
URLs returned by the `canonicalize()` method.

* Add a `containingUrl` field to the `canonicalize()` and `findFileUrl()`
methods of importers, which is set to the canonical URL of the stylesheet that
contains the current load. For filesystem importers, this is always set; for
other importers, it's set only if the current load has no URL scheme, or if
its URL scheme is declared as non-canonical by the importer.

### Dart API

* Add `AsyncImporter.isNonCanonicalScheme`, which importers (async or sync) can
use to indicate that a certain URL scheme will never be used for URLs returned
by the `canonicalize()` method.

* Add `AsyncImporter.containingUrl`, which is set during calls to the
`canonicalize()` method to the canonical URL of the stylesheet that contains
the current load. This is set only if the current load has no URL scheme, or
if its URL scheme is declared as non-canonical by the importer.

### Embedded Sass

* The `CalculationValue.interpolation` field is deprecated and will be removed
in a future version. It will no longer be set by the compiler, and if the host
sets it it will be treated as equivalent to `CalculationValue.string` except
that `"("` and `")"` will be added to the beginning and end of the string
values.

* Properly include TypeScript types in the `sass-embedded` package.

## 1.67.0

* All functions defined in CSS Values and Units 4 are now once again parsed as
calculation objects: `round()`, `mod()`, `rem()`, `sin()`, `cos()`, `tan()`,
`asin()`, `acos()`, `atan()`, `atan2()`, `pow()`, `sqrt()`, `hypot()`,
`log()`, `exp()`, `abs()`, and `sign()`.

Unlike in 1.65.0, function calls are _not_ locked into being parsed as
calculations or plain Sass functions at parse-time. This means that
user-defined functions will take precedence over CSS calculations of the same
name. Although the function names `calc()` and `clamp()` are still forbidden,
users may continue to freely define functions whose names overlap with other
CSS calculations (including `abs()`, `min()`, `max()`, and `round()` whose
names overlap with global Sass functions).

* **Breaking change**: As a consequence of the change in calculation parsing
described above, calculation functions containing interpolation are now parsed
more strictly than before. However, _almost_ all interpolations that would
have produced valid CSS will continue to work. The only exception is
`#{$variable}%` which is not valid in Sass and is no longer valid in
calculations. Instead of this, either use `$variable` directly and ensure it
already has the `%` unit, or write `($variable * 1%)`.

* **Potentially breaking bug fix**: The importer used to load a given file is no
longer used to load absolute URLs that appear in that file. This was
unintented behavior that contradicted the Sass specification. Absolute URLs
will now correctly be loaded only from the global importer list. This applies
to the modern JS API, the Dart API, and the embedded protocol.

### Embedded Sass

* Substantially improve the embedded compiler's performance when compiling many
files or files that require many importer or function call round-trips with
the embedded host.

## 1.66.1

### JS API
Expand Down
39 changes: 31 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
We'd love to accept your patches and contributions to this project. There are
just a few small guidelines you need to follow.

* [Contributor License Agreement](#contributor-license-agreement)
* [Code Reviews](#code-reviews)
* [Large Language Models](#large-language-models)
* [Release Process](#release-process)
* [Keeping in Sync With Other Packages](#keeping-in-sync-with-other-packages)
* [Local Development](#local-development)
* [Continuous Integration](#continuous-integration)
* [Release](#release)

## Contributor License Agreement

Contributions to this project must be accompanied by a Contributor License
Expand All @@ -15,13 +24,24 @@ You generally only need to submit a CLA once, so if you've already submitted one
(even if it was for a different project), you probably don't need to do it
again.

## Code reviews
## Code Reviews

All submissions, including submissions by project members, require review. We
use GitHub pull requests for this purpose. Consult
[GitHub Help](https://help.github.com/articles/about-pull-requests/) for more
information on using pull requests.

## Large Language Models

Do not submit any code or prose written or modified by large language models or
"artificial intelligence" such as GitHub Copilot or ChatGPT to this project.
These tools produce code that looks plausible, which means that not only is it
likely to contain bugs those bugs are likely to be difficult to notice on
review. In addition, because these models were trained indiscriminately and
non-consensually on open-source code with a variety of licenses, it's not
obvious that we have the moral or legal right to redistribute code they
generate.

## Release process

Because this package's version remains in lockstep with the current version of
Expand All @@ -36,7 +56,7 @@ such, manual commits should never:
* Update the `package.json`'s `"compiler-version"` field to a non-`-dev` number.
Changing it from non-`-dev` to dev when using a new feature is fine.

# Keeping in Sync With Other Packages
## Keeping in Sync With Other Packages

The embedded host depends on several different components which come from
different repositories:
Expand All @@ -51,12 +71,12 @@ different repositories:

These dependencies are made available in different ways depending on context.

## Local Development
### Local Development

When developing locally, you can download all of these dependencies by running
`npm run init`. This provides the following options for `compiler` (for the
embedded compiler), `protocol` (for the embedded protocol), and `api` (for the
JS API):
`npm install` and then `npm run init`. This provides the following options for
`compiler` (for the embedded compiler), `protocol` (for the embedded protocol),
and `api` (for the JS API):

* `--<type>-path`: The local filesystem path of the package to use. This is
useful when doing local development on both the host and its dependencies at
Expand All @@ -65,6 +85,9 @@ JS API):
* `--<type>-ref`: A Git reference for the GitHub repository of the package to
clone.

If developing locally, you will need to specify both the compiler and language.
For example: `npm run init -- --compiler-path=dart-sass --language-path=language`.

By default:

* This uses the version of the embedded protocol and compiler specified by
Expand All @@ -77,14 +100,14 @@ By default:
* This uses the Dart Sass version from the latest revision on GitHub, unless the
`--compiler-path` was passed in which case it uses that version of Dart Sass.

## Continuous Integration
### Continuous Integration

CI tests also use `npm run init`, so they use the same defaults as local
development. However, if the pull request description includes a link to a pull
request for Dart Sass, the embedded protocol, or the JS API, this will check out
that version and run tests against it instead.

## Release
### Release

When this package is released to npm, it downloads the embedded protocol version
that matches `protocol-version` in `package.json`. It downloads the latest JS
Expand Down
5 changes: 5 additions & 0 deletions lib/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const SassBoolean = sass.SassBoolean;
export const SassCalculation = sass.SassCalculation
export const SassColor = sass.SassColor;
export const SassFunction = sass.SassFunction;
export const SassMixin = sass.SassMixin;
export const SassList = sass.SassList;
export const SassMap = sass.SassMap;
export const SassNumber = sass.SassNumber;
Expand Down Expand Up @@ -95,6 +96,10 @@ export default {
defaultExportDeprecation();
return sass.SassFunction;
},
get SassMixin() {
defaultExportDeprecation();
return sass.SassMixin;
},
get SassList() {
defaultExportDeprecation();
return sass.SassList;
Expand Down
1 change: 1 addition & 0 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export {sassFalse, sassTrue} from './src/value/boolean';
export {SassColor} from './src/value/color';
export {SassFunction} from './src/value/function';
export {SassMap} from './src/value/map';
export {SassMixin} from './src/value/mixin';
export {SassNumber} from './src/value/number';
export {SassString} from './src/value/string';
export {Value} from './src/value';
Expand Down
62 changes: 45 additions & 17 deletions lib/src/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,32 @@ import {MessageTransformer} from './message-transformer';
import {PacketTransformer} from './packet-transformer';
import {SyncEmbeddedCompiler} from './sync-compiler';
import {deprotofySourceSpan} from './deprotofy-span';
import {legacyImporterProtocol} from './legacy/importer';
import {
removeLegacyImporter,
removeLegacyImporterFromSpan,
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<sync extends 'sync' | 'async'> = Options<sync> & {
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<sync extends 'sync' | 'async'> =
StringOptions<sync> & {legacy?: boolean};

export function compile(
path: string,
options?: Options<'sync'>
options?: OptionsWithLegacy<'sync'>
): CompileResult {
const importers = new ImporterRegistry(options);
return compileRequestSync(
Expand All @@ -34,7 +55,7 @@ export function compile(

export function compileString(
source: string,
options?: StringOptions<'sync'>
options?: StringOptionsWithLegacy<'sync'>
): CompileResult {
const importers = new ImporterRegistry(options);
return compileRequestSync(
Expand All @@ -46,7 +67,7 @@ export function compileString(

export function compileAsync(
path: string,
options?: Options<'async'>
options?: OptionsWithLegacy<'async'>
): Promise<CompileResult> {
const importers = new ImporterRegistry(options);
return compileRequestAsync(
Expand All @@ -58,7 +79,7 @@ export function compileAsync(

export function compileStringAsync(
source: string,
options?: StringOptions<'async'>
options?: StringOptionsWithLegacy<'async'>
): Promise<CompileResult> {
const importers = new ImporterRegistry(options);
return compileRequestAsync(
Expand Down Expand Up @@ -151,7 +172,7 @@ function newCompileRequest(
async function compileRequestAsync(
request: proto.InboundMessage_CompileRequest,
importers: ImporterRegistry<'async'>,
options?: Options<'async'>
options?: OptionsWithLegacy<'async'> & {legacy?: boolean}
): Promise<CompileResult> {
const functions = new FunctionRegistry(options?.functions);
const embeddedCompiler = new AsyncEmbeddedCompiler();
Expand Down Expand Up @@ -197,7 +218,7 @@ async function compileRequestAsync(
function compileRequestSync(
request: proto.InboundMessage_CompileRequest,
importers: ImporterRegistry<'sync'>,
options?: Options<'sync'>
options?: OptionsWithLegacy<'sync'>
): CompileResult {
const functions = new FunctionRegistry(options?.functions);
const embeddedCompiler = new SyncEmbeddedCompiler();
Expand Down Expand Up @@ -272,33 +293,40 @@ function createDispatcher<sync extends 'sync' | 'async'>(

/** Handles a log event according to `options`. */
function handleLogEvent(
options: Options<'sync' | 'async'> | undefined,
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(event.message, {
span: deprotofySourceSpan(event.span!),
options.logger.debug(message, {
span: span!,
});
} else {
console.error(event.formatted);
console.error(formatted);
}
} else {
if (options?.logger?.warn) {
const params: {deprecation: boolean; span?: SourceSpan; stack?: string} =
{
deprecation: event.type === proto.LogEventType.DEPRECATION_WARNING,
};

const spanProto = event.span;
if (spanProto) params.span = deprotofySourceSpan(spanProto);
if (span) params.span = span;

const stack = event.stackTrace;
if (stack) params.stack = stack;
if (stack) {
params.stack = options?.legacy ? removeLegacyImporter(stack) : stack;
}

options.logger.warn(event.message, params);
options.logger.warn(message, params);
} else {
console.error(event.formatted);
console.error(formatted);
}
}
}
Expand Down
18 changes: 14 additions & 4 deletions lib/src/importer-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
register(
importer: Importer<sync> | FileImporter<sync>
): proto.InboundMessage_CompileRequest_Importer {
const response = new proto.InboundMessage_CompileRequest_Importer();
const message = new proto.InboundMessage_CompileRequest_Importer();
if ('canonicalize' in importer) {
if ('findFileUrl' in importer) {
throw new Error(
Expand All @@ -54,14 +54,18 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
);
}

response.importer = {case: 'importerId', value: this.id};
message.importer = {case: 'importerId', value: this.id};
message.nonCanonicalScheme =
typeof importer.nonCanonicalScheme === 'string'
? [importer.nonCanonicalScheme]
: importer.nonCanonicalScheme ?? [];
this.importersById.set(this.id, importer);
} else {
response.importer = {case: 'fileImporterId', value: this.id};
message.importer = {case: 'fileImporterId', value: this.id};
this.fileImportersById.set(this.id, importer);
}
this.id += 1;
return response;
return message;
}

/** Handles a canonicalization request. */
Expand All @@ -78,6 +82,9 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
return thenOr(
importer.canonicalize(request.url, {
fromImport: request.fromImport,
containingUrl: request.containingUrl
? new URL(request.containingUrl)
: null,
}),
url =>
new proto.InboundMessage_CanonicalizeResponse({
Expand Down Expand Up @@ -157,6 +164,9 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
return thenOr(
importer.findFileUrl(request.url, {
fromImport: request.fromImport,
containingUrl: request.containingUrl
? new URL(request.containingUrl)
: null,
}),
url => {
if (!url) return new proto.InboundMessage_FileImportResponse();
Expand Down
Loading
Loading