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.

Sign up for GitHub

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

Avoid parsing feature-set defaults at module initialization #683

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Avoid parsing feature-set defaults at module initialization #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c68cd88
Avoid parsing feature-set defaults at module initialization
timostamm Jan 29, 2024
d396d05
Don't export getFeatureSetDefaults
timostamm Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions packages/protobuf/src/create-descriptor-set.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ import {
parseTextFormatEnumValue,
parseTextFormatScalarValue,
} from "./private/text-format.js";
import type { BinaryReadOptions, BinaryWriteOptions } from "./binary-format.js";
import type { FeatureResolverFn } from "./private/feature-set.js";
import {
createFeatureResolver,
featureSetDefaults,
} from "./private/feature-set.js";
import { createFeatureResolver } from "./private/feature-set.js";

/**
* Create a DescriptorSet, a convenient interface for working with a set of
Expand Down Expand Up @@ -90,8 +88,9 @@ export function createDescriptorSet(
let resolveFeatures = resolverByEdition.get(edition);
if (resolveFeatures === undefined) {
resolveFeatures = createFeatureResolver(
options?.featureSetDefaults ?? featureSetDefaults,
edition,
options?.featureSetDefaults,
options?.serializationOptions,
);
resolverByEdition.set(edition, resolveFeatures);
}
Expand All @@ -117,6 +116,12 @@ interface CreateDescriptorSetOptions {
* `--experimental_edition_defaults_out`.
*/
featureSetDefaults?: FeatureSetDefaults;

/**
* Internally, data is serialized when features are resolved. The
* serialization options given here will be used for feature resolution.
*/
serializationOptions?: Partial<BinaryReadOptions & BinaryWriteOptions>;
}

/**
Expand Down
43 changes: 28 additions & 15 deletions packages/protobuf/src/private/feature-set.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,24 @@ import {
FeatureSetDefaults,
} from "../google/protobuf/descriptor_pb.js";
import { protoBase64 } from "../proto-base64.js";
import type {
BinaryReadOptions,
BinaryWriteOptions,
} from "../binary-format.js";

/**
* Static edition feature defaults supported by @bufbuild/protobuf.
* Return the edition feature defaults supported by @bufbuild/protobuf.
*/
export const featureSetDefaults = FeatureSetDefaults.fromBinary(
protoBase64.dec(
/*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/,
),
);
function getFeatureSetDefaults(
options?: Partial<BinaryReadOptions>,
): FeatureSetDefaults {
return FeatureSetDefaults.fromBinary(
protoBase64.dec(
/*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/,
),
options,
);
}

/**
* A merged google.protobuf.FeaturesSet, with all fields guaranteed to be set.
Expand All @@ -46,18 +55,22 @@ export type FeatureResolverFn = (
) => MergedFeatureSet;

/**
* Create an edition feature resolver with the given feature set defaults.
* Create an edition feature resolver with the given feature set defaults, or
* the feature set defaults supported by @bufbuild/protobuf.
*/
export function createFeatureResolver(
compiledFeatureSetDefaults: FeatureSetDefaults,
edition: Edition,
compiledFeatureSetDefaults?: FeatureSetDefaults,
serializationOptions?: Partial<BinaryReadOptions & BinaryWriteOptions>,
): FeatureResolverFn {
const min = compiledFeatureSetDefaults.minimumEdition;
const max = compiledFeatureSetDefaults.maximumEdition;
const fds =
compiledFeatureSetDefaults ?? getFeatureSetDefaults(serializationOptions);
const min = fds.minimumEdition;
const max = fds.maximumEdition;
if (
min === undefined ||
max === undefined ||
compiledFeatureSetDefaults.defaults.some((d) => d.edition === undefined)
fds.defaults.some((d) => d.edition === undefined)
) {
throw new Error("Invalid FeatureSetDefaults");
}
Expand All @@ -72,7 +85,7 @@ export function createFeatureResolver(
);
}
let highestMatch: { e: Edition; f: FeatureSet } | undefined = undefined;
for (const c of compiledFeatureSetDefaults.defaults) {
for (const c of fds.defaults) {
const e = c.edition ?? 0;
if (e > edition) {
continue;
Expand All @@ -88,12 +101,12 @@ export function createFeatureResolver(
if (highestMatch === undefined) {
throw new Error(`No valid default found for edition ${Edition[edition]}`);
}
const defaultsBin = highestMatch.f.toBinary();
const featureSetBin = highestMatch.f.toBinary(serializationOptions);
return (...rest): MergedFeatureSet => {
const f = FeatureSet.fromBinary(defaultsBin);
const f = FeatureSet.fromBinary(featureSetBin, serializationOptions);
for (const c of rest) {
if (c !== undefined) {
f.fromBinary(c.toBinary());
f.fromBinary(c.toBinary(serializationOptions), serializationOptions);
}
}
if (!validateMergedFeatures(f)) {
Expand Down
Loading