From 873e42497a4c89883b9c921d543d8793a27183ce Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 11:51:40 +0200 Subject: [PATCH 1/9] frontend-internal: add OpaqueType helper Signed-off-by: Patrik Oldsberg --- .changeset/funny-rocks-train.md | 6 + .../src/wiring/InternalExtensionDefinition.ts | 52 +++--- .../src/wiring/OpaqueType.ts | 164 ++++++++++++++++++ .../frontend-internal/src/wiring/index.ts | 5 +- .../src/wiring/createExtension.ts | 48 ++--- .../wiring/createExtensionBlueprint.test.tsx | 6 +- .../src/wiring/createFrontendModule.ts | 9 +- .../src/wiring/createFrontendPlugin.ts | 9 +- .../src/wiring/resolveExtensionDefinition.ts | 4 +- .../src/app/createExtensionTester.tsx | 6 +- 10 files changed, 230 insertions(+), 79 deletions(-) create mode 100644 .changeset/funny-rocks-train.md create mode 100644 packages/frontend-internal/src/wiring/OpaqueType.ts diff --git a/.changeset/funny-rocks-train.md b/.changeset/funny-rocks-train.md new file mode 100644 index 0000000000..f5b2fbba63 --- /dev/null +++ b/.changeset/funny-rocks-train.md @@ -0,0 +1,6 @@ +--- +'@backstage/frontend-plugin-api': patch +'@backstage/frontend-test-utils': patch +--- + +Internal refactor of usage of opaque types. diff --git a/packages/frontend-internal/src/wiring/InternalExtensionDefinition.ts b/packages/frontend-internal/src/wiring/InternalExtensionDefinition.ts index 840e45ff4a..a77ffd485b 100644 --- a/packages/frontend-internal/src/wiring/InternalExtensionDefinition.ts +++ b/packages/frontend-internal/src/wiring/InternalExtensionDefinition.ts @@ -25,19 +25,19 @@ import { PortableSchema, ResolvedExtensionInputs, } from '@backstage/frontend-plugin-api'; +import { OpaqueType } from './OpaqueType'; -export type InternalExtensionDefinition< - T extends ExtensionDefinitionParameters = ExtensionDefinitionParameters, -> = ExtensionDefinition & { - readonly kind?: string; - readonly namespace?: string; - readonly name?: string; - readonly attachTo: { id: string; input: string }; - readonly disabled: boolean; - readonly configSchema?: PortableSchema; -} & ( +export const OpaqueExtensionDefinition = OpaqueType.create<{ + public: ExtensionDefinition; + versions: | { readonly version: 'v1'; + readonly kind?: string; + readonly namespace?: string; + readonly name?: string; + readonly attachTo: { id: string; input: string }; + readonly disabled: boolean; + readonly configSchema?: PortableSchema; readonly inputs: { [inputName in string]: { $$type: '@backstage/ExtensionInput'; @@ -63,6 +63,12 @@ export type InternalExtensionDefinition< } | { readonly version: 'v2'; + readonly kind?: string; + readonly namespace?: string; + readonly name?: string; + readonly attachTo: { id: string; input: string }; + readonly disabled: boolean; + readonly configSchema?: PortableSchema; readonly inputs: { [inputName in string]: ExtensionInput< AnyExtensionDataRef, @@ -81,24 +87,8 @@ export type InternalExtensionDefinition< >; }>; }): Iterable>; - } - ); - -/** @internal */ -export function toInternalExtensionDefinition< - T extends ExtensionDefinitionParameters, ->(overrides: ExtensionDefinition): InternalExtensionDefinition { - const internal = overrides as InternalExtensionDefinition; - if (internal.$$type !== '@backstage/ExtensionDefinition') { - throw new Error( - `Invalid extension definition instance, bad type '${internal.$$type}'`, - ); - } - const version = internal.version; - if (version !== 'v1' && version !== 'v2') { - throw new Error( - `Invalid extension definition instance, bad version '${version}'`, - ); - } - return internal; -} + }; +}>({ + type: '@backstage/ExtensionDefinition', + versions: ['v1', 'v2'], +}); diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts new file mode 100644 index 0000000000..868bca3315 --- /dev/null +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -0,0 +1,164 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// TODO(Rugvip): This lives here temporarily, but should be moved to a more +// central location. It's useful for backend packages too so we'll need to have +// it in a common package, but it might also be that we want to make it +// available publicly too in which case it would make sense to have this be part +// of @backstage/version-bridge. The problem with exporting it from there is +// that it would need to be very stable at that point, so it might be a bit +// early to put it there already. + +/** + * A helper for working with opaque types. + */ +export class OpaqueType< + T extends { + public: { $$type: string }; + versions: { version: string }; + }, +> { + /** + * Creates a new opaque type. + * + * @param options.type The type identifier of the opaque type + * @param options.versions The available versions of the opaque type + * @returns A new opaque type helper + */ + static create< + T extends { + public: { $$type: string }; + versions: { version: string }; + }, + >(options: { + type: T['public']['$$type']; + versions: Array; + }) { + return new OpaqueType(options.type, new Set(options.versions)); + } + + #type: string; + #versions: Set; + + private constructor(type: string, versions: Set) { + this.#type = type; + this.#versions = versions; + } + + /** + * The internal version of the opaque type, used like this: `typeof MyOpaqueType.TPublic` + * + * @remarks + * + * This property is only useful for type checking, its runtime value is `undefined`. + */ + TPublic: T['public'] = undefined as any; + + /** + * The internal version of the opaque type, used like this: `typeof MyOpaqueType.TInternal` + * + * @remarks + * + * This property is only useful for type checking, its runtime value is `undefined`. + */ + TInternal: T['public'] & T['versions'] = undefined as any; + + /** + * @param value Input value expected to be an instance of this opaque type + * @throws If the value is not an instance of this opaque type + * @returns The internal version of the opaque type + */ + toInternal(value: unknown): T['public'] & T['versions'] { + if (!this.#isThisType(value)) { + throw new Error( + `Invalid opaque type, expected '${ + this.#type + }', but got '${this.#stringifyUnknown(value)}'`, + ); + } + this.#throwIfInvalidVersion(value.version); + return value; + } + + /** + * @param value Input value expected to be an instance of this opaque type + * @returns True if the value matches this opaque type + */ + isInternal(value: unknown): value is T['public'] & T['versions'] { + if (!this.#isThisType(value)) { + return false; + } + this.#throwIfInvalidVersion(value.version); + return true; + } + + /** + * @param version The expected version of the opaque type + * @param value Input value expected to be an instance of this opaque type + * @returns True if the value matches this opaque type and is the expected version + */ + isVersion( + version: TVersion, + value: unknown, + ): value is T['public'] & + (T['versions'] extends infer UVersion + ? UVersion extends { version: TVersion } + ? UVersion + : never + : never) { + return this.#isThisType(value) && value.version === version; + } + + /** + * Creates an instance of the opaque type, returning the public public type. + * + * By providing a type argument you can narrow the return to specific type parameters. + */ + create( + value: T['public'] & T['versions'] & Object, // & Object to allow for object properties too, e.g. toString() + ): TBase { + return value as unknown as TBase; + } + + #throwIfInvalidVersion(version: string) { + if (!this.#versions.has(version)) { + const versionsStr = Array.from(this.#versions).join("', '"); + throw new Error( + `Invalid opaque type instance, bad version '${version}', expected one of '${versionsStr}'`, + ); + } + } + + #isThisType(value: unknown): value is T['public'] & T['versions'] { + if (value === null || typeof value !== 'object') { + return false; + } + return (value as T['public']).$$type === this.#type; + } + + #stringifyUnknown(value: unknown) { + if (typeof value !== 'object') { + return `<${typeof value}>`; + } + if (value === null) { + return ''; + } + if ('$$type' in value) { + return String(value.$$type); + } + return String(value); + } +} diff --git a/packages/frontend-internal/src/wiring/index.ts b/packages/frontend-internal/src/wiring/index.ts index d0aefbed64..86f1ed9ab7 100644 --- a/packages/frontend-internal/src/wiring/index.ts +++ b/packages/frontend-internal/src/wiring/index.ts @@ -14,7 +14,4 @@ * limitations under the License. */ -export { - toInternalExtensionDefinition, - type InternalExtensionDefinition, -} from './InternalExtensionDefinition'; +export { OpaqueExtensionDefinition } from './InternalExtensionDefinition'; diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 7aef3fc1b5..78e66dffe5 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -31,7 +31,7 @@ import { import { ExtensionInput } from './createExtensionInput'; import { z } from 'zod'; import { createSchemaFromZod } from '../schema/createSchemaFromZod'; -import { InternalExtensionDefinition } from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; /** * Convert a single extension input into a matching resolved input. @@ -366,26 +366,6 @@ export function createExtension< namespace: string | undefined extends TNamespace ? undefined : TNamespace; name: string | undefined extends TName ? undefined : TName; }> { - type T = { - config: string extends keyof TConfigSchema - ? {} - : { - [key in keyof TConfigSchema]: z.infer>; - }; - configInput: string extends keyof TConfigSchema - ? {} - : z.input< - z.ZodObject<{ - [key in keyof TConfigSchema]: ReturnType; - }> - >; - output: UOutput; - inputs: TInputs; - kind: string | undefined extends TKind ? undefined : TKind; - namespace: string | undefined extends TNamespace ? undefined : TNamespace; - name: string | undefined extends TName ? undefined : TName; - }; - const schemaDeclaration = options.config?.schema; const configSchema = schemaDeclaration && @@ -397,10 +377,30 @@ export function createExtension< ), ); - return { + return OpaqueExtensionDefinition.create({ $$type: '@backstage/ExtensionDefinition', version: 'v2', - T: undefined as unknown as T, + T: undefined as unknown as { + config: string extends keyof TConfigSchema + ? {} + : { + [key in keyof TConfigSchema]: z.infer< + ReturnType + >; + }; + configInput: string extends keyof TConfigSchema + ? {} + : z.input< + z.ZodObject<{ + [key in keyof TConfigSchema]: ReturnType; + }> + >; + output: UOutput; + inputs: TInputs; + kind: string | undefined extends TKind ? undefined : TKind; + namespace: string | undefined extends TNamespace ? undefined : TNamespace; + name: string | undefined extends TName ? undefined : TName; + }, kind: options.kind, namespace: options.namespace, name: options.name, @@ -512,5 +512,5 @@ export function createExtension< }, }) as ExtensionDefinition; }, - } as InternalExtensionDefinition; + }); } diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionBlueprint.test.tsx b/packages/frontend-plugin-api/src/wiring/createExtensionBlueprint.test.tsx index 47ef7a75b4..16c763d725 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtensionBlueprint.test.tsx +++ b/packages/frontend-plugin-api/src/wiring/createExtensionBlueprint.test.tsx @@ -29,12 +29,12 @@ import { createExtensionInput } from './createExtensionInput'; import { RouteRef } from '../routing'; import { ExtensionDefinition } from './createExtension'; import { createExtensionDataContainer } from './createExtensionDataContainer'; -import { toInternalExtensionDefinition } from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; function unused(..._any: any[]) {} function factoryOutput(ext: ExtensionDefinition, inputs: unknown = undefined) { - const int = toInternalExtensionDefinition(ext); + const int = OpaqueExtensionDefinition.toInternal(ext); if (int.version !== 'v2') { throw new Error('Expected v2 extension'); } @@ -680,7 +680,7 @@ describe('createExtensionBlueprint', () => { }, }); - const ext = toInternalExtensionDefinition( + const ext = OpaqueExtensionDefinition.toInternal( blueprint.makeWithOverrides({ output: [testDataRef2], factory(origFactory) { diff --git a/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts b/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts index e3fa8b6682..bece1539fa 100644 --- a/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts +++ b/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts @@ -14,10 +14,7 @@ * limitations under the License. */ -import { - InternalExtensionDefinition, - toInternalExtensionDefinition, -} from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; import { ExtensionDefinition } from './createExtension'; import { Extension, @@ -58,11 +55,11 @@ export function createFrontendModule< const extensions = new Array>(); const extensionDefinitionsById = new Map< string, - InternalExtensionDefinition + typeof OpaqueExtensionDefinition.TInternal >(); for (const def of options.extensions ?? []) { - const internal = toInternalExtensionDefinition(def); + const internal = OpaqueExtensionDefinition.toInternal(def); const ext = resolveExtensionDefinition(def, { namespace: pluginId }); extensions.push(ext); extensionDefinitionsById.set(ext.id, { diff --git a/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts b/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts index adcd2b0867..865eae6444 100644 --- a/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts +++ b/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts @@ -14,10 +14,7 @@ * limitations under the License. */ -import { - InternalExtensionDefinition, - toInternalExtensionDefinition, -} from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; import { ExtensionDefinition } from './createExtension'; import { Extension, @@ -96,11 +93,11 @@ export function createFrontendPlugin< const extensions = new Array>(); const extensionDefinitionsById = new Map< string, - InternalExtensionDefinition + typeof OpaqueExtensionDefinition.TInternal >(); for (const def of options.extensions ?? []) { - const internal = toInternalExtensionDefinition(def); + const internal = OpaqueExtensionDefinition.toInternal(def); const ext = resolveExtensionDefinition(def, { namespace: options.id }); extensions.push(ext); extensionDefinitionsById.set(ext.id, { diff --git a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts index a377cde2b9..53de20bff2 100644 --- a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts +++ b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts @@ -26,7 +26,7 @@ import { AnyExtensionDataRef, ExtensionDataValue, } from './createExtensionDataRef'; -import { toInternalExtensionDefinition } from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; /** @public */ export interface Extension { @@ -141,7 +141,7 @@ export function resolveExtensionDefinition< definition: ExtensionDefinition, context?: { namespace?: string }, ): Extension { - const internalDefinition = toInternalExtensionDefinition(definition); + const internalDefinition = OpaqueExtensionDefinition.toInternal(definition); const { name, kind, diff --git a/packages/frontend-test-utils/src/app/createExtensionTester.tsx b/packages/frontend-test-utils/src/app/createExtensionTester.tsx index 5eda2d6e11..37ec1d8fd9 100644 --- a/packages/frontend-test-utils/src/app/createExtensionTester.tsx +++ b/packages/frontend-test-utils/src/app/createExtensionTester.tsx @@ -37,7 +37,7 @@ import { instantiateAppNodeTree } from '../../../frontend-app-api/src/tree/insta // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { readAppExtensionsConfig } from '../../../frontend-app-api/src/tree/readAppExtensionsConfig'; import { TestApiRegistry } from '@backstage/test-utils'; -import { toInternalExtensionDefinition } from '@internal/frontend'; +import { OpaqueExtensionDefinition } from '@internal/frontend'; /** @public */ export class ExtensionQuery { @@ -105,7 +105,7 @@ export class ExtensionTester { ); } - const { name, namespace } = toInternalExtensionDefinition(extension); + const { name, namespace } = OpaqueExtensionDefinition.toInternal(extension); const definition = { ...extension, @@ -143,7 +143,7 @@ export class ExtensionTester { const tree = this.#resolveTree(); // Same fallback logic as in .add - const { name, namespace } = toInternalExtensionDefinition(extension); + const { name, namespace } = OpaqueExtensionDefinition.toInternal(extension); const definition = { ...extension, name: !namespace && !name ? 'test' : name, From 43f0d76b7b88dcc4fc13931fea9d4bc5e7a90c60 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 12:18:01 +0200 Subject: [PATCH 2/9] frontend-internal: add tests for OpaqueType Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 265 ++++++++++++++++++ 1 file changed, 265 insertions(+) create mode 100644 packages/frontend-internal/src/wiring/OpaqueType.test.ts diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts new file mode 100644 index 0000000000..052e9dc041 --- /dev/null +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -0,0 +1,265 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { OpaqueType } from './OpaqueType'; + +describe('OpaqueType', () => { + it('should create a basic opaque type with a single version', () => { + type MyType = { + $$type: 'my-type'; + }; + + const OpaqueMyType = OpaqueType.create<{ + public: MyType; + versions: { + version: 'v1'; + foo: string; + }; + }>({ + type: 'my-type', + versions: ['v1'], + }); + + OpaqueMyType.create({ + // @ts-expect-error - wrong type + $$type: 'wrong-type', + version: 'v1', + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + // @ts-expect-error - unsupported version + version: 'v2', + foo: 'bar', + }); + + // @ts-expect-error - missing version + OpaqueMyType.create({ + $$type: 'my-type', + foo: 'bar', + }); + + // @ts-expect-error - missing internal field + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + // @ts-expect-error - invalid internal field + foo: 3, + }); + + const myInstance = OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + foo: 'bar', + }); + + expect(myInstance.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstance.version).toBe('v1'); + // @ts-expect-error - internal field not accessible + expect(myInstance.foo).toBe('bar'); + + expect(OpaqueMyType.isInternal(myInstance)).toBe(true); + + const myInternal = OpaqueMyType.toInternal(myInstance); + expect(myInternal).toBe(myInstance); + // All fields accessible + expect(myInternal.$$type).toBe('my-type'); + expect(myInternal.version).toBe('v1'); + expect(myInternal.foo).toBe('bar'); + + expect(OpaqueMyType.isVersion('v1', myInstance)).toBe(true); + expect(OpaqueMyType.isVersion('v2' as any, myInstance)).toBe(false); + + expect(OpaqueMyType.isInternal('hello')).toBe(false); + expect(OpaqueMyType.isVersion('v1', 'hello')).toBe(false); + + expect(() => + OpaqueMyType.toInternal('hello'), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => OpaqueMyType.toInternal(3)).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => + OpaqueMyType.toInternal(undefined), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => + OpaqueMyType.toInternal(Symbol('wat')), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => + OpaqueMyType.toInternal(null), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => + OpaqueMyType.toInternal(() => {}), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got ''"`, + ); + expect(() => + OpaqueMyType.toInternal({ $$type: 'some-other-type' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got 'some-other-type'"`, + ); + expect(() => + OpaqueMyType.toInternal({ an: 'object' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type, expected 'my-type', but got '[object Object]'"`, + ); + }); + + it('should create a basic opaque type with multiple versions', () => { + type MyType = { + $$type: 'my-type'; + }; + + const OpaqueMyType = OpaqueType.create<{ + public: MyType; + versions: + | { + version: 'v1'; + foo: string; + } + | { + version: 'v2'; + bar: string; + }; + }>({ + type: 'my-type', + versions: ['v1', 'v2'], + }); + + OpaqueMyType.create({ + // @ts-expect-error - wrong type + $$type: 'wrong-type', + version: 'v1', + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + // @ts-expect-error - unsupported version + version: 'v3', + foo: 'bar', + }); + + // @ts-expect-error - missing version + OpaqueMyType.create({ + $$type: 'my-type', + foo: 'bar', + }); + + // @ts-expect-error - missing internal field + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + // @ts-expect-error - invalid internal field + foo: 3, + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v2', + // @ts-expect-error - version mismatch + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + // @ts-expect-error - version mismatch + bar: 'foo', + }); + + const myInstanceV1 = OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + foo: 'bar', + }); + + const myInstanceV2 = OpaqueMyType.create({ + $$type: 'my-type', + version: 'v2', + bar: 'foo', + }); + + expect(myInstanceV1.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstanceV1.version).toBe('v1'); + // @ts-expect-error - internal field not accessible + expect(myInstanceV1.foo).toBe('bar'); + + expect(myInstanceV2.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstanceV2.version).toBe('v2'); + // @ts-expect-error - internal field not accessible + expect(myInstanceV2.bar).toBe('foo'); + + expect(OpaqueMyType.isInternal(myInstanceV1)).toBe(true); + expect(OpaqueMyType.isInternal(myInstanceV2)).toBe(true); + + const myInternalV1 = OpaqueMyType.toInternal(myInstanceV1); + expect(myInternalV1).toBe(myInstanceV1); + // All fields accessible + expect(myInternalV1.$$type).toBe('my-type'); + expect(myInternalV1.version).toBe('v1'); + // @ts-expect-error - version has not been narrowed down + expect(myInternalV1.foo).toBe('bar'); + + const myInternalV2 = OpaqueMyType.toInternal(myInstanceV2); + expect(myInternalV2).toBe(myInstanceV2); + // All fields accessible + expect(myInternalV2.$$type).toBe('my-type'); + expect(myInternalV2.version).toBe('v2'); + // @ts-expect-error - version has not been narrowed down + expect(myInternalV2.bar).toBe('foo'); + + // Narrowing the version allows access to internal fields + expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); + expect(myInternalV2.version === 'v2' && myInternalV2.bar).toBe('foo'); + + expect(OpaqueMyType.isVersion('v1', myInstanceV1)).toBe(true); + expect(OpaqueMyType.isVersion('v2', myInstanceV1)).toBe(false); + + expect(OpaqueMyType.isVersion('v1', myInstanceV2)).toBe(false); + expect(OpaqueMyType.isVersion('v2', myInstanceV2)).toBe(true); + + // Narrowing the version allows access to internal fields + expect(OpaqueMyType.isVersion('v1', myInstanceV1) && myInstanceV1.foo).toBe( + 'bar', + ); + expect(OpaqueMyType.isVersion('v2', myInstanceV2) && myInstanceV2.bar).toBe( + 'foo', + ); + }); +}); From b17ad1b745f54e2ce4c0edc5aa563ac1d6e8cca8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 12:57:37 +0200 Subject: [PATCH 3/9] frontend-internal: add support for undefined version for opaque types Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 200 ++++++++++++++++++ .../src/wiring/OpaqueType.ts | 27 ++- 2 files changed, 218 insertions(+), 9 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index 052e9dc041..8e0ab0a3c8 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -262,4 +262,204 @@ describe('OpaqueType', () => { 'foo', ); }); + + it('should support undefined version for backwards compatibility', () => { + type MyType = { + $$type: 'my-type'; + }; + + const OpaqueMyType = OpaqueType.create<{ + public: MyType; + versions: { + version: undefined; + foo: string; + }; + }>({ + type: 'my-type', + versions: [undefined], + }); + + OpaqueMyType.create({ + // @ts-expect-error - wrong type + $$type: 'wrong-type', + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + // @ts-expect-error - unsupported version + version: 'v1', + foo: 'bar', + }); + + // @ts-expect-error - missing internal field + OpaqueMyType.create({ + $$type: 'my-type', + version: undefined, + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: undefined, + // @ts-expect-error - invalid internal field + foo: 3, + }); + + const myInstance = OpaqueMyType.create({ + $$type: 'my-type', + version: undefined, + foo: 'bar', + }); + + expect(myInstance.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstance.version).toBe(undefined); + // @ts-expect-error - internal field not accessible + expect(myInstance.foo).toBe('bar'); + + expect(OpaqueMyType.isInternal(myInstance)).toBe(true); + + const myInternal = OpaqueMyType.toInternal(myInstance); + expect(myInternal).toBe(myInstance); + // All fields accessible + expect(myInternal.$$type).toBe('my-type'); + expect(myInternal.version).toBe(undefined); + expect(myInternal.foo).toBe('bar'); + + expect(OpaqueMyType.isVersion(undefined, myInstance)).toBe(true); + expect(OpaqueMyType.isVersion('v1' as any, myInstance)).toBe(false); + + expect(OpaqueMyType.isInternal('hello')).toBe(false); + expect(OpaqueMyType.isVersion(undefined, 'hello')).toBe(false); + }); + + it('should support undefined version mixed with defined versions', () => { + type MyType = { + $$type: 'my-type'; + }; + + const OpaqueMyType = OpaqueType.create<{ + public: MyType; + versions: + | { + version: 'v1'; + foo: string; + } + | { + version: undefined; + bar: string; + }; + }>({ + type: 'my-type', + versions: ['v1', undefined], + }); + + OpaqueMyType.create({ + // @ts-expect-error - wrong type + $$type: 'wrong-type', + version: 'v1', + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + // @ts-expect-error - unsupported version + version: 'v3', + foo: 'bar', + }); + + // @ts-expect-error - missing version + OpaqueMyType.create({ + $$type: 'my-type', + foo: 'bar', + }); + + // @ts-expect-error - missing internal field + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + // @ts-expect-error - invalid internal field + foo: 3, + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: undefined, + // @ts-expect-error - version mismatch + foo: 'bar', + }); + + OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + // @ts-expect-error - version mismatch + bar: 'foo', + }); + + const myInstanceV1 = OpaqueMyType.create({ + $$type: 'my-type', + version: 'v1', + foo: 'bar', + }); + + const myInstanceV2 = OpaqueMyType.create({ + $$type: 'my-type', + version: undefined, + bar: 'foo', + }); + + expect(myInstanceV1.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstanceV1.version).toBe('v1'); + // @ts-expect-error - internal field not accessible + expect(myInstanceV1.foo).toBe('bar'); + + expect(myInstanceV2.$$type).toBe('my-type'); + // @ts-expect-error - version field not accessible + expect(myInstanceV2.version).toBe(undefined); + // @ts-expect-error - internal field not accessible + expect(myInstanceV2.bar).toBe('foo'); + + expect(OpaqueMyType.isInternal(myInstanceV1)).toBe(true); + expect(OpaqueMyType.isInternal(myInstanceV2)).toBe(true); + + const myInternalV1 = OpaqueMyType.toInternal(myInstanceV1); + expect(myInternalV1).toBe(myInstanceV1); + // All fields accessible + expect(myInternalV1.$$type).toBe('my-type'); + expect(myInternalV1.version).toBe('v1'); + // @ts-expect-error - version has not been narrowed down + expect(myInternalV1.foo).toBe('bar'); + + const myInternalV2 = OpaqueMyType.toInternal(myInstanceV2); + expect(myInternalV2).toBe(myInstanceV2); + // All fields accessible + expect(myInternalV2.$$type).toBe('my-type'); + expect(myInternalV2.version).toBe(undefined); + // @ts-expect-error - version has not been narrowed down + expect(myInternalV2.bar).toBe('foo'); + + // Narrowing the version allows access to internal fields + expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); + expect(myInternalV2.version === undefined && myInternalV2.bar).toBe('foo'); + + expect(OpaqueMyType.isVersion('v1', myInstanceV1)).toBe(true); + expect(OpaqueMyType.isVersion(undefined, myInstanceV1)).toBe(false); + + expect(OpaqueMyType.isVersion('v1', myInstanceV2)).toBe(false); + expect(OpaqueMyType.isVersion(undefined, myInstanceV2)).toBe(true); + + // Narrowing the version allows access to internal fields + expect(OpaqueMyType.isVersion('v1', myInstanceV1) && myInstanceV1.foo).toBe( + 'bar', + ); + expect( + OpaqueMyType.isVersion(undefined, myInstanceV2) && myInstanceV2.bar, + ).toBe('foo'); + }); }); diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index 868bca3315..332843f6cc 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -28,7 +28,7 @@ export class OpaqueType< T extends { public: { $$type: string }; - versions: { version: string }; + versions: { version?: string } & Object; }, > { /** @@ -41,7 +41,7 @@ export class OpaqueType< static create< T extends { public: { $$type: string }; - versions: { version: string }; + versions: { version?: string } & Object; }, >(options: { type: T['public']['$$type']; @@ -51,9 +51,9 @@ export class OpaqueType< } #type: string; - #versions: Set; + #versions: Set; - private constructor(type: string, versions: Set) { + private constructor(type: string, versions: Set) { this.#type = type; this.#versions = versions; } @@ -83,7 +83,7 @@ export class OpaqueType< */ toInternal(value: unknown): T['public'] & T['versions'] { if (!this.#isThisType(value)) { - throw new Error( + throw new TypeError( `Invalid opaque type, expected '${ this.#type }', but got '${this.#stringifyUnknown(value)}'`, @@ -133,11 +133,20 @@ export class OpaqueType< return value as unknown as TBase; } - #throwIfInvalidVersion(version: string) { + #throwIfInvalidVersion(version: string | undefined) { if (!this.#versions.has(version)) { - const versionsStr = Array.from(this.#versions).join("', '"); - throw new Error( - `Invalid opaque type instance, bad version '${version}', expected one of '${versionsStr}'`, + const expected = []; + if (this.#versions.has(undefined)) { + expected.push('undefined'); + } + const versions = Array.from(this.#versions).filter(Boolean); + if (versions.length > 0) { + expected.push(`one of ['${versions.join("', '")}']`); + } + throw new TypeError( + `Invalid opaque type instance, got version '${version}', expected ${expected.join( + ' or ', + )}`, ); } } From 8c7a8d41d236e28ac37f29ff7524a84e1ef43811 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 13:14:30 +0200 Subject: [PATCH 4/9] frontend-internal: refactor OpaqueType to just isType + toInternal Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 56 +++---------- .../src/wiring/OpaqueType.ts | 79 +++++++------------ 2 files changed, 38 insertions(+), 97 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index 8e0ab0a3c8..eada974bbf 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -78,7 +78,8 @@ describe('OpaqueType', () => { // @ts-expect-error - internal field not accessible expect(myInstance.foo).toBe('bar'); - expect(OpaqueMyType.isInternal(myInstance)).toBe(true); + expect(OpaqueMyType.isType(myInstance)).toBe(true); + expect(OpaqueMyType.isType('hello')).toBe(false); const myInternal = OpaqueMyType.toInternal(myInstance); expect(myInternal).toBe(myInstance); @@ -87,12 +88,6 @@ describe('OpaqueType', () => { expect(myInternal.version).toBe('v1'); expect(myInternal.foo).toBe('bar'); - expect(OpaqueMyType.isVersion('v1', myInstance)).toBe(true); - expect(OpaqueMyType.isVersion('v2' as any, myInstance)).toBe(false); - - expect(OpaqueMyType.isInternal('hello')).toBe(false); - expect(OpaqueMyType.isVersion('v1', 'hello')).toBe(false); - expect(() => OpaqueMyType.toInternal('hello'), ).toThrowErrorMatchingInlineSnapshot( @@ -225,8 +220,9 @@ describe('OpaqueType', () => { // @ts-expect-error - internal field not accessible expect(myInstanceV2.bar).toBe('foo'); - expect(OpaqueMyType.isInternal(myInstanceV1)).toBe(true); - expect(OpaqueMyType.isInternal(myInstanceV2)).toBe(true); + expect(OpaqueMyType.isType(myInstanceV1)).toBe(true); + expect(OpaqueMyType.isType(myInstanceV2)).toBe(true); + expect(OpaqueMyType.isType('hello')).toBe(false); const myInternalV1 = OpaqueMyType.toInternal(myInstanceV1); expect(myInternalV1).toBe(myInstanceV1); @@ -247,20 +243,6 @@ describe('OpaqueType', () => { // Narrowing the version allows access to internal fields expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); expect(myInternalV2.version === 'v2' && myInternalV2.bar).toBe('foo'); - - expect(OpaqueMyType.isVersion('v1', myInstanceV1)).toBe(true); - expect(OpaqueMyType.isVersion('v2', myInstanceV1)).toBe(false); - - expect(OpaqueMyType.isVersion('v1', myInstanceV2)).toBe(false); - expect(OpaqueMyType.isVersion('v2', myInstanceV2)).toBe(true); - - // Narrowing the version allows access to internal fields - expect(OpaqueMyType.isVersion('v1', myInstanceV1) && myInstanceV1.foo).toBe( - 'bar', - ); - expect(OpaqueMyType.isVersion('v2', myInstanceV2) && myInstanceV2.bar).toBe( - 'foo', - ); }); it('should support undefined version for backwards compatibility', () => { @@ -317,7 +299,8 @@ describe('OpaqueType', () => { // @ts-expect-error - internal field not accessible expect(myInstance.foo).toBe('bar'); - expect(OpaqueMyType.isInternal(myInstance)).toBe(true); + expect(OpaqueMyType.isType(myInstance)).toBe(true); + expect(OpaqueMyType.isType('hello')).toBe(false); const myInternal = OpaqueMyType.toInternal(myInstance); expect(myInternal).toBe(myInstance); @@ -325,12 +308,6 @@ describe('OpaqueType', () => { expect(myInternal.$$type).toBe('my-type'); expect(myInternal.version).toBe(undefined); expect(myInternal.foo).toBe('bar'); - - expect(OpaqueMyType.isVersion(undefined, myInstance)).toBe(true); - expect(OpaqueMyType.isVersion('v1' as any, myInstance)).toBe(false); - - expect(OpaqueMyType.isInternal('hello')).toBe(false); - expect(OpaqueMyType.isVersion(undefined, 'hello')).toBe(false); }); it('should support undefined version mixed with defined versions', () => { @@ -425,8 +402,9 @@ describe('OpaqueType', () => { // @ts-expect-error - internal field not accessible expect(myInstanceV2.bar).toBe('foo'); - expect(OpaqueMyType.isInternal(myInstanceV1)).toBe(true); - expect(OpaqueMyType.isInternal(myInstanceV2)).toBe(true); + expect(OpaqueMyType.isType(myInstanceV1)).toBe(true); + expect(OpaqueMyType.isType(myInstanceV2)).toBe(true); + expect(OpaqueMyType.isType('hello')).toBe(false); const myInternalV1 = OpaqueMyType.toInternal(myInstanceV1); expect(myInternalV1).toBe(myInstanceV1); @@ -447,19 +425,5 @@ describe('OpaqueType', () => { // Narrowing the version allows access to internal fields expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); expect(myInternalV2.version === undefined && myInternalV2.bar).toBe('foo'); - - expect(OpaqueMyType.isVersion('v1', myInstanceV1)).toBe(true); - expect(OpaqueMyType.isVersion(undefined, myInstanceV1)).toBe(false); - - expect(OpaqueMyType.isVersion('v1', myInstanceV2)).toBe(false); - expect(OpaqueMyType.isVersion(undefined, myInstanceV2)).toBe(true); - - // Narrowing the version allows access to internal fields - expect(OpaqueMyType.isVersion('v1', myInstanceV1) && myInstanceV1.foo).toBe( - 'bar', - ); - expect( - OpaqueMyType.isVersion(undefined, myInstanceV2) && myInstanceV2.bar, - ).toBe('foo'); }); }); diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index 332843f6cc..5da3003418 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -28,7 +28,7 @@ export class OpaqueType< T extends { public: { $$type: string }; - versions: { version?: string } & Object; + versions: { version: string | undefined }; }, > { /** @@ -41,7 +41,7 @@ export class OpaqueType< static create< T extends { public: { $$type: string }; - versions: { version?: string } & Object; + versions: { version: string | undefined }; }, >(options: { type: T['public']['$$type']; @@ -78,48 +78,43 @@ export class OpaqueType< /** * @param value Input value expected to be an instance of this opaque type - * @throws If the value is not an instance of this opaque type + * @returns True if the value matches this opaque type + */ + isType(value: unknown): value is T['public'] { + return this.#isThisInternalType(value); + } + + /** + * @param value Input value expected to be an instance of this opaque type + * @throws If the value is not an instance of this opaque type or is of an unsupported version * @returns The internal version of the opaque type */ toInternal(value: unknown): T['public'] & T['versions'] { - if (!this.#isThisType(value)) { + if (!this.#isThisInternalType(value)) { throw new TypeError( `Invalid opaque type, expected '${ this.#type }', but got '${this.#stringifyUnknown(value)}'`, ); } - this.#throwIfInvalidVersion(value.version); - return value; - } - /** - * @param value Input value expected to be an instance of this opaque type - * @returns True if the value matches this opaque type - */ - isInternal(value: unknown): value is T['public'] & T['versions'] { - if (!this.#isThisType(value)) { - return false; + if (!this.#versions.has(value.version)) { + const expected = []; + if (this.#versions.has(undefined)) { + expected.push('undefined'); + } + const versions = Array.from(this.#versions).filter(Boolean); + if (versions.length > 0) { + expected.push(`one of ['${versions.join("', '")}']`); + } + throw new TypeError( + `Invalid opaque type instance, got version '${ + value.version + }', expected ${expected.join(' or ')}`, + ); } - this.#throwIfInvalidVersion(value.version); - return true; - } - /** - * @param version The expected version of the opaque type - * @param value Input value expected to be an instance of this opaque type - * @returns True if the value matches this opaque type and is the expected version - */ - isVersion( - version: TVersion, - value: unknown, - ): value is T['public'] & - (T['versions'] extends infer UVersion - ? UVersion extends { version: TVersion } - ? UVersion - : never - : never) { - return this.#isThisType(value) && value.version === version; + return value; } /** @@ -133,25 +128,7 @@ export class OpaqueType< return value as unknown as TBase; } - #throwIfInvalidVersion(version: string | undefined) { - if (!this.#versions.has(version)) { - const expected = []; - if (this.#versions.has(undefined)) { - expected.push('undefined'); - } - const versions = Array.from(this.#versions).filter(Boolean); - if (versions.length > 0) { - expected.push(`one of ['${versions.join("', '")}']`); - } - throw new TypeError( - `Invalid opaque type instance, got version '${version}', expected ${expected.join( - ' or ', - )}`, - ); - } - } - - #isThisType(value: unknown): value is T['public'] & T['versions'] { + #isThisInternalType(value: unknown): value is T['public'] & T['versions'] { if (value === null || typeof value !== 'object') { return false; } From 0313cdac633a09cd37f2667ab19ef5d579a61a9e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 13:22:25 +0200 Subject: [PATCH 5/9] frontend-internal: few more tests for opaqueType.toInternal Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index eada974bbf..10ca93bfa8 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -80,6 +80,8 @@ describe('OpaqueType', () => { expect(OpaqueMyType.isType(myInstance)).toBe(true); expect(OpaqueMyType.isType('hello')).toBe(false); + expect(OpaqueMyType.isType({ $$type: 'some-other' })).toBe(false); + expect(OpaqueMyType.isType({ $$type: 'my-type' })).toBe(true); const myInternal = OpaqueMyType.toInternal(myInstance); expect(myInternal).toBe(myInstance); @@ -126,6 +128,21 @@ describe('OpaqueType', () => { ).toThrowErrorMatchingInlineSnapshot( `"Invalid opaque type, expected 'my-type', but got '[object Object]'"`, ); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'undefined', expected one of ['v1']"`, + ); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'v3', expected one of ['v1']"`, + ); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type', version: { foo: 'bar' } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version '[object Object]', expected one of ['v1']"`, + ); }); it('should create a basic opaque type with multiple versions', () => { @@ -243,6 +260,17 @@ describe('OpaqueType', () => { // Narrowing the version allows access to internal fields expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); expect(myInternalV2.version === 'v2' && myInternalV2.bar).toBe('foo'); + + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'undefined', expected one of ['v1', 'v2']"`, + ); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'v3', expected one of ['v1', 'v2']"`, + ); }); it('should support undefined version for backwards compatibility', () => { @@ -308,6 +336,13 @@ describe('OpaqueType', () => { expect(myInternal.$$type).toBe('my-type'); expect(myInternal.version).toBe(undefined); expect(myInternal.foo).toBe('bar'); + + expect(OpaqueMyType.toInternal({ $$type: 'my-type' })).toBeDefined(); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'v3', expected undefined"`, + ); }); it('should support undefined version mixed with defined versions', () => { @@ -425,5 +460,12 @@ describe('OpaqueType', () => { // Narrowing the version allows access to internal fields expect(myInternalV1.version === 'v1' && myInternalV1.foo).toBe('bar'); expect(myInternalV2.version === undefined && myInternalV2.bar).toBe('foo'); + + expect(OpaqueMyType.toInternal({ $$type: 'my-type' })).toBeDefined(); + expect(() => + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid opaque type instance, got version 'v3', expected undefined or one of ['v1']"`, + ); }); }); From 375c8383352b7f53a64df56fb7909c1a09be5010 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 13:31:02 +0200 Subject: [PATCH 6/9] frontend-internal: cleaner messaging for invalid opaque type versions Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 30 +++++++++++-------- .../src/wiring/OpaqueType.ts | 22 +++++++------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index 10ca93bfa8..edf23cd732 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -131,17 +131,17 @@ describe('OpaqueType', () => { expect(() => OpaqueMyType.toInternal({ $$type: 'my-type' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'undefined', expected one of ['v1']"`, + `"Invalid opaque type instance, got version undefined, expected 'v1'"`, ); expect(() => - OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v0' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'v3', expected one of ['v1']"`, + `"Invalid opaque type instance, got version 'v0', expected 'v1'"`, ); expect(() => OpaqueMyType.toInternal({ $$type: 'my-type', version: { foo: 'bar' } }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version '[object Object]', expected one of ['v1']"`, + `"Invalid opaque type instance, got version '[object Object]', expected 'v1'"`, ); }); @@ -160,10 +160,14 @@ describe('OpaqueType', () => { | { version: 'v2'; bar: string; + } + | { + version: 'v3'; + baz: string; }; }>({ type: 'my-type', - versions: ['v1', 'v2'], + versions: ['v1', 'v2', 'v3'], }); OpaqueMyType.create({ @@ -176,7 +180,7 @@ describe('OpaqueType', () => { OpaqueMyType.create({ $$type: 'my-type', // @ts-expect-error - unsupported version - version: 'v3', + version: 'v0', foo: 'bar', }); @@ -264,12 +268,12 @@ describe('OpaqueType', () => { expect(() => OpaqueMyType.toInternal({ $$type: 'my-type' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'undefined', expected one of ['v1', 'v2']"`, + `"Invalid opaque type instance, got version undefined, expected 'v1', 'v2', or 'v3'"`, ); expect(() => - OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v0' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'v3', expected one of ['v1', 'v2']"`, + `"Invalid opaque type instance, got version 'v0', expected 'v1', 'v2', or 'v3'"`, ); }); @@ -339,9 +343,9 @@ describe('OpaqueType', () => { expect(OpaqueMyType.toInternal({ $$type: 'my-type' })).toBeDefined(); expect(() => - OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), + OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v0' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'v3', expected undefined"`, + `"Invalid opaque type instance, got version 'v0', expected undefined"`, ); }); @@ -363,7 +367,7 @@ describe('OpaqueType', () => { }; }>({ type: 'my-type', - versions: ['v1', undefined], + versions: [undefined, 'v1'], }); OpaqueMyType.create({ @@ -465,7 +469,7 @@ describe('OpaqueType', () => { expect(() => OpaqueMyType.toInternal({ $$type: 'my-type', version: 'v3' }), ).toThrowErrorMatchingInlineSnapshot( - `"Invalid opaque type instance, got version 'v3', expected undefined or one of ['v1']"`, + `"Invalid opaque type instance, got version 'v3', expected undefined or 'v1'"`, ); }); }); diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index 5da3003418..5a13529388 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -99,18 +99,16 @@ export class OpaqueType< } if (!this.#versions.has(value.version)) { - const expected = []; - if (this.#versions.has(undefined)) { - expected.push('undefined'); - } - const versions = Array.from(this.#versions).filter(Boolean); - if (versions.length > 0) { - expected.push(`one of ['${versions.join("', '")}']`); + const versions = Array.from(this.#versions).map(this.#stringifyVersion); + if (versions.length > 1) { + versions[versions.length - 1] = `or ${versions[versions.length - 1]}`; } + const expected = + versions.length > 2 ? versions.join(', ') : versions.join(' '); throw new TypeError( - `Invalid opaque type instance, got version '${ - value.version - }', expected ${expected.join(' or ')}`, + `Invalid opaque type instance, got version ${this.#stringifyVersion( + value.version, + )}, expected ${expected}`, ); } @@ -147,4 +145,8 @@ export class OpaqueType< } return String(value); } + + #stringifyVersion = (version: string | undefined) => { + return version ? `'${version}'` : 'undefined'; + }; } From 4cfacd936bbaf32eaecefa706a181f5642e20ded Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 13:33:53 +0200 Subject: [PATCH 7/9] frontend-internal: opaqueType.create -> .createInstance Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 58 +++++++++---------- .../src/wiring/OpaqueType.ts | 2 +- .../src/wiring/createExtension.ts | 2 +- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index edf23cd732..f952f9cdbb 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -33,14 +33,14 @@ describe('OpaqueType', () => { versions: ['v1'], }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ // @ts-expect-error - wrong type $$type: 'wrong-type', version: 'v1', foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', // @ts-expect-error - unsupported version version: 'v2', @@ -48,25 +48,25 @@ describe('OpaqueType', () => { }); // @ts-expect-error - missing version - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', // @ts-expect-error - invalid internal field foo: 3, }); - const myInstance = OpaqueMyType.create({ + const myInstance = OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', foo: 'bar', @@ -170,14 +170,14 @@ describe('OpaqueType', () => { versions: ['v1', 'v2', 'v3'], }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ // @ts-expect-error - wrong type $$type: 'wrong-type', version: 'v1', foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', // @ts-expect-error - unsupported version version: 'v0', @@ -185,45 +185,45 @@ describe('OpaqueType', () => { }); // @ts-expect-error - missing version - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', // @ts-expect-error - invalid internal field foo: 3, }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v2', // @ts-expect-error - version mismatch foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', // @ts-expect-error - version mismatch bar: 'foo', }); - const myInstanceV1 = OpaqueMyType.create({ + const myInstanceV1 = OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', foo: 'bar', }); - const myInstanceV2 = OpaqueMyType.create({ + const myInstanceV2 = OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v2', bar: 'foo', @@ -293,13 +293,13 @@ describe('OpaqueType', () => { versions: [undefined], }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ // @ts-expect-error - wrong type $$type: 'wrong-type', foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', // @ts-expect-error - unsupported version version: 'v1', @@ -307,19 +307,19 @@ describe('OpaqueType', () => { }); // @ts-expect-error - missing internal field - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: undefined, }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: undefined, // @ts-expect-error - invalid internal field foo: 3, }); - const myInstance = OpaqueMyType.create({ + const myInstance = OpaqueMyType.createInstance({ $$type: 'my-type', version: undefined, foo: 'bar', @@ -370,14 +370,14 @@ describe('OpaqueType', () => { versions: [undefined, 'v1'], }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ // @ts-expect-error - wrong type $$type: 'wrong-type', version: 'v1', foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', // @ts-expect-error - unsupported version version: 'v3', @@ -385,45 +385,45 @@ describe('OpaqueType', () => { }); // @ts-expect-error - missing version - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', // @ts-expect-error - invalid internal field foo: 3, }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: undefined, // @ts-expect-error - version mismatch foo: 'bar', }); - OpaqueMyType.create({ + OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', // @ts-expect-error - version mismatch bar: 'foo', }); - const myInstanceV1 = OpaqueMyType.create({ + const myInstanceV1 = OpaqueMyType.createInstance({ $$type: 'my-type', version: 'v1', foo: 'bar', }); - const myInstanceV2 = OpaqueMyType.create({ + const myInstanceV2 = OpaqueMyType.createInstance({ $$type: 'my-type', version: undefined, bar: 'foo', diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index 5a13529388..5f571a5d4b 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -120,7 +120,7 @@ export class OpaqueType< * * By providing a type argument you can narrow the return to specific type parameters. */ - create( + createInstance( value: T['public'] & T['versions'] & Object, // & Object to allow for object properties too, e.g. toString() ): TBase { return value as unknown as TBase; diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 78e66dffe5..92c8bc7be9 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -377,7 +377,7 @@ export function createExtension< ), ); - return OpaqueExtensionDefinition.create({ + return OpaqueExtensionDefinition.createInstance({ $$type: '@backstage/ExtensionDefinition', version: 'v2', T: undefined as unknown as { From bb0ea523b9b885d2530bd9e6fac34928e6868b63 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 14:04:39 +0200 Subject: [PATCH 8/9] frontend-internal: switch opaqueType.createInstance to decorate + select version Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.test.ts | 179 ++++++------------ .../src/wiring/OpaqueType.ts | 25 ++- .../src/wiring/createExtension.ts | 48 +++-- 3 files changed, 102 insertions(+), 150 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.test.ts b/packages/frontend-internal/src/wiring/OpaqueType.test.ts index f952f9cdbb..2e49ca9029 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.test.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.test.ts @@ -33,42 +33,20 @@ describe('OpaqueType', () => { versions: ['v1'], }); - OpaqueMyType.createInstance({ - // @ts-expect-error - wrong type - $$type: 'wrong-type', - version: 'v1', - foo: 'bar', - }); - - OpaqueMyType.createInstance({ - $$type: 'my-type', - // @ts-expect-error - unsupported version - version: 'v2', - foo: 'bar', - }); - - // @ts-expect-error - missing version - OpaqueMyType.createInstance({ - $$type: 'my-type', + // @ts-expect-error - unsupported version + OpaqueMyType.createInstance('v2', { foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', - }); + OpaqueMyType.createInstance('v1', {}); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + OpaqueMyType.createInstance('v1', { // @ts-expect-error - invalid internal field foo: 3, }); - const myInstance = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + const myInstance = OpaqueMyType.createInstance('v1', { foo: 'bar', }); @@ -170,62 +148,34 @@ describe('OpaqueType', () => { versions: ['v1', 'v2', 'v3'], }); - OpaqueMyType.createInstance({ - // @ts-expect-error - wrong type - $$type: 'wrong-type', - version: 'v1', - foo: 'bar', - }); - - OpaqueMyType.createInstance({ - $$type: 'my-type', - // @ts-expect-error - unsupported version - version: 'v0', - foo: 'bar', - }); - - // @ts-expect-error - missing version - OpaqueMyType.createInstance({ - $$type: 'my-type', + // @ts-expect-error - unsupported version + OpaqueMyType.createInstance('v0', { foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', - }); + OpaqueMyType.createInstance('v1', {}); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + OpaqueMyType.createInstance('v1', { // @ts-expect-error - invalid internal field foo: 3, }); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v2', + OpaqueMyType.createInstance('v2', { // @ts-expect-error - version mismatch foo: 'bar', }); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + OpaqueMyType.createInstance('v1', { // @ts-expect-error - version mismatch bar: 'foo', }); - const myInstanceV1 = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + const myInstanceV1 = OpaqueMyType.createInstance('v1', { foo: 'bar', }); - const myInstanceV2 = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v2', + const myInstanceV2 = OpaqueMyType.createInstance('v2', { bar: 'foo', }); @@ -293,35 +243,20 @@ describe('OpaqueType', () => { versions: [undefined], }); - OpaqueMyType.createInstance({ - // @ts-expect-error - wrong type - $$type: 'wrong-type', - foo: 'bar', - }); - - OpaqueMyType.createInstance({ - $$type: 'my-type', - // @ts-expect-error - unsupported version - version: 'v1', + // @ts-expect-error - unsupported version + OpaqueMyType.createInstance('v1', { foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: undefined, - }); + OpaqueMyType.createInstance(undefined, {}); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: undefined, + OpaqueMyType.createInstance(undefined, { // @ts-expect-error - invalid internal field foo: 3, }); - const myInstance = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: undefined, + const myInstance = OpaqueMyType.createInstance(undefined, { foo: 'bar', }); @@ -370,62 +305,34 @@ describe('OpaqueType', () => { versions: [undefined, 'v1'], }); - OpaqueMyType.createInstance({ - // @ts-expect-error - wrong type - $$type: 'wrong-type', - version: 'v1', - foo: 'bar', - }); - - OpaqueMyType.createInstance({ - $$type: 'my-type', - // @ts-expect-error - unsupported version - version: 'v3', - foo: 'bar', - }); - - // @ts-expect-error - missing version - OpaqueMyType.createInstance({ - $$type: 'my-type', + // @ts-expect-error - unsupported version + OpaqueMyType.createInstance('v0', { foo: 'bar', }); // @ts-expect-error - missing internal field - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', - }); + OpaqueMyType.createInstance('v1', {}); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + OpaqueMyType.createInstance('v1', { // @ts-expect-error - invalid internal field foo: 3, }); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: undefined, + OpaqueMyType.createInstance(undefined, { // @ts-expect-error - version mismatch foo: 'bar', }); - OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + OpaqueMyType.createInstance('v1', { // @ts-expect-error - version mismatch bar: 'foo', }); - const myInstanceV1 = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: 'v1', + const myInstanceV1 = OpaqueMyType.createInstance('v1', { foo: 'bar', }); - const myInstanceV2 = OpaqueMyType.createInstance({ - $$type: 'my-type', - version: undefined, + const myInstanceV2 = OpaqueMyType.createInstance(undefined, { bar: 'foo', }); @@ -472,4 +379,38 @@ describe('OpaqueType', () => { `"Invalid opaque type instance, got version 'v3', expected undefined or 'v1'"`, ); }); + + it('should create an empty opaque type with no versions', () => { + type MyType = { + $$type: 'my-type'; + }; + + const OpaqueMyType = OpaqueType.create<{ + public: MyType; + versions: { + version: undefined; + }; + }>({ + type: 'my-type', + versions: [undefined], + }); + + // @ts-expect-error - unsupported version + OpaqueMyType.createInstance('v0', { + foo: 'bar', + }); + + const myInstance = OpaqueMyType.createInstance(undefined, {}); + + expect(myInstance.$$type).toBe('my-type'); + + expect(OpaqueMyType.isType(myInstance)).toBe(true); + expect(OpaqueMyType.isType('hello')).toBe(false); + + const myInternal = OpaqueMyType.toInternal(myInstance); + expect(myInternal).toBe(myInstance); + // All fields accessible + expect(myInternal.$$type).toBe('my-type'); + expect(myInternal.version).toBe(undefined); + }); }); diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index 5f571a5d4b..abd74bd910 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -116,14 +116,27 @@ export class OpaqueType< } /** - * Creates an instance of the opaque type, returning the public public type. + * Creates an instance of the opaque type, returning the public type. * - * By providing a type argument you can narrow the return to specific type parameters. + * @param version The version of the instance to create + * @param value The remaining public and internal properties of the instance + * @returns An instance of the opaque type */ - createInstance( - value: T['public'] & T['versions'] & Object, // & Object to allow for object properties too, e.g. toString() - ): TBase { - return value as unknown as TBase; + createInstance( + version: TVersion, + props: Omit & + (T['versions'] extends infer UVersion + ? UVersion extends { version: TVersion } + ? Omit + : never + : never) & + Object, // & Object to allow for object properties too, e.g. toString() + ): T['public'] { + return { + ...(props as object), + $$type: this.#type, + ...(version && { version }), + } as T['public']; } #isThisInternalType(value: unknown): value is T['public'] & T['versions'] { diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 92c8bc7be9..4941324bed 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -366,6 +366,26 @@ export function createExtension< namespace: string | undefined extends TNamespace ? undefined : TNamespace; name: string | undefined extends TName ? undefined : TName; }> { + type T = { + config: string extends keyof TConfigSchema + ? {} + : { + [key in keyof TConfigSchema]: z.infer>; + }; + configInput: string extends keyof TConfigSchema + ? {} + : z.input< + z.ZodObject<{ + [key in keyof TConfigSchema]: ReturnType; + }> + >; + output: UOutput; + inputs: TInputs; + kind: string | undefined extends TKind ? undefined : TKind; + namespace: string | undefined extends TNamespace ? undefined : TNamespace; + name: string | undefined extends TName ? undefined : TName; + }; + const schemaDeclaration = options.config?.schema; const configSchema = schemaDeclaration && @@ -377,30 +397,8 @@ export function createExtension< ), ); - return OpaqueExtensionDefinition.createInstance({ - $$type: '@backstage/ExtensionDefinition', - version: 'v2', - T: undefined as unknown as { - config: string extends keyof TConfigSchema - ? {} - : { - [key in keyof TConfigSchema]: z.infer< - ReturnType - >; - }; - configInput: string extends keyof TConfigSchema - ? {} - : z.input< - z.ZodObject<{ - [key in keyof TConfigSchema]: ReturnType; - }> - >; - output: UOutput; - inputs: TInputs; - kind: string | undefined extends TKind ? undefined : TKind; - namespace: string | undefined extends TNamespace ? undefined : TNamespace; - name: string | undefined extends TName ? undefined : TName; - }, + return OpaqueExtensionDefinition.createInstance('v2', { + T: undefined as unknown as T, kind: options.kind, namespace: options.namespace, name: options.name, @@ -512,5 +510,5 @@ export function createExtension< }, }) as ExtensionDefinition; }, - }); + }) as ExtensionDefinition; } From dadb50658653e4a92bfcd32611863cf1adf4b82c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Sep 2024 17:19:47 +0200 Subject: [PATCH 9/9] frontend-internal: loosen type of opaqueType.createInstance return value Signed-off-by: Patrik Oldsberg --- .../src/wiring/OpaqueType.ts | 9 ++-- .../src/wiring/createExtension.ts | 44 +++++++++---------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/frontend-internal/src/wiring/OpaqueType.ts b/packages/frontend-internal/src/wiring/OpaqueType.ts index abd74bd910..3d05b619bc 100644 --- a/packages/frontend-internal/src/wiring/OpaqueType.ts +++ b/packages/frontend-internal/src/wiring/OpaqueType.ts @@ -122,7 +122,10 @@ export class OpaqueType< * @param value The remaining public and internal properties of the instance * @returns An instance of the opaque type */ - createInstance( + createInstance< + TVersion extends T['versions']['version'], + TPublic extends T['public'], + >( version: TVersion, props: Omit & (T['versions'] extends infer UVersion @@ -131,12 +134,12 @@ export class OpaqueType< : never : never) & Object, // & Object to allow for object properties too, e.g. toString() - ): T['public'] { + ): TPublic { return { ...(props as object), $$type: this.#type, ...(version && { version }), - } as T['public']; + } as unknown as TPublic; } #isThisInternalType(value: unknown): value is T['public'] & T['versions'] { diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 4941324bed..ac7b1de432 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -366,26 +366,6 @@ export function createExtension< namespace: string | undefined extends TNamespace ? undefined : TNamespace; name: string | undefined extends TName ? undefined : TName; }> { - type T = { - config: string extends keyof TConfigSchema - ? {} - : { - [key in keyof TConfigSchema]: z.infer>; - }; - configInput: string extends keyof TConfigSchema - ? {} - : z.input< - z.ZodObject<{ - [key in keyof TConfigSchema]: ReturnType; - }> - >; - output: UOutput; - inputs: TInputs; - kind: string | undefined extends TKind ? undefined : TKind; - namespace: string | undefined extends TNamespace ? undefined : TNamespace; - name: string | undefined extends TName ? undefined : TName; - }; - const schemaDeclaration = options.config?.schema; const configSchema = schemaDeclaration && @@ -398,7 +378,27 @@ export function createExtension< ); return OpaqueExtensionDefinition.createInstance('v2', { - T: undefined as unknown as T, + T: undefined as unknown as { + config: string extends keyof TConfigSchema + ? {} + : { + [key in keyof TConfigSchema]: z.infer< + ReturnType + >; + }; + configInput: string extends keyof TConfigSchema + ? {} + : z.input< + z.ZodObject<{ + [key in keyof TConfigSchema]: ReturnType; + }> + >; + output: UOutput; + inputs: TInputs; + kind: string | undefined extends TKind ? undefined : TKind; + namespace: string | undefined extends TNamespace ? undefined : TNamespace; + name: string | undefined extends TName ? undefined : TName; + }, kind: options.kind, namespace: options.namespace, name: options.name, @@ -510,5 +510,5 @@ export function createExtension< }, }) as ExtensionDefinition; }, - }) as ExtensionDefinition; + }); }