From 2bf859e0f151caa3c1efaa2499206e02cf62c6ca Mon Sep 17 00:00:00 2001 From: eyelidlessness Date: Thu, 27 Feb 2025 15:30:06 -0800 Subject: [PATCH] engine (fix): Forms load with any `jr:preload` or `jr:preloadParams` value Addresses the current failure to load the IFRC Distribution Site Assessment fixture in Web Forms Preview. --- .changeset/six-moose-leave.md | 5 + .../UnknownPreloadAttributeValueNotice.ts | 35 ------- .../src/parse/model/BindPreloadDefinition.ts | 94 ++++++------------- 3 files changed, 33 insertions(+), 101 deletions(-) create mode 100644 .changeset/six-moose-leave.md delete mode 100644 packages/xforms-engine/src/error/UnknownPreloadAttributeValueNotice.ts diff --git a/.changeset/six-moose-leave.md b/.changeset/six-moose-leave.md new file mode 100644 index 00000000..bab0240f --- /dev/null +++ b/.changeset/six-moose-leave.md @@ -0,0 +1,5 @@ +--- +'@getodk/xforms-engine': patch +--- + +Fix: relax parsing of `jr:preload` and `jr:preloadParams`. Any value for either attribute is accepted. Known (specified in ODK XForms, at time of writing) values are provided as type hints, similarly to how known appearances are specified. diff --git a/packages/xforms-engine/src/error/UnknownPreloadAttributeValueNotice.ts b/packages/xforms-engine/src/error/UnknownPreloadAttributeValueNotice.ts deleted file mode 100644 index 59660de2..00000000 --- a/packages/xforms-engine/src/error/UnknownPreloadAttributeValueNotice.ts +++ /dev/null @@ -1,35 +0,0 @@ -type PreloadAttributeName = 'jr:preload' | 'jr:preloadParams'; - -/** - * @todo This class is intentionally named to reflect the fact that it is not - * intended to indefinitely block loading a form! Insofar as we currently throw - * this error, the intent is to determine whether we have gaps in our support - * for - * {@link https://getodk.github.io/xforms-spec/#preload-attributes | preload attributes}. - * - * @todo Open question(s) for design around the broader error production story: - * how should we design for conditions which are _optionally errors_ (e.g. - * varying levels of strictness, use case-specific cases where certain kinds of - * errors aren't failures)? In particular, how should we design for: - * - * - Categorization that allows selecting which error conditions are applied, at - * what level of severity? - * - Blocking progress on failure-level severity, proceeding on sub-failure - * severity? - * - * Question applies to this case where we may want to error for unknown preload - * attribute values in dev/test, but may not want to error under most (all?) - * user-facing conditions. - */ -export class UnknownPreloadAttributeValueNotice extends Error { - constructor( - attributeName: PreloadAttributeName, - expectedValues: ReadonlyArray, - unknownValue: string | null - ) { - const expected = expectedValues.map((value) => JSON.stringify(value)).join(', '); - super( - `Unknown ${attributeName} value. Expected one of ${expected}, got: ${JSON.stringify(unknownValue)}` - ); - } -} diff --git a/packages/xforms-engine/src/parse/model/BindPreloadDefinition.ts b/packages/xforms-engine/src/parse/model/BindPreloadDefinition.ts index 23d32e19..585e68a7 100644 --- a/packages/xforms-engine/src/parse/model/BindPreloadDefinition.ts +++ b/packages/xforms-engine/src/parse/model/BindPreloadDefinition.ts @@ -1,69 +1,29 @@ import { JAVAROSA_NAMESPACE_URI } from '@getodk/common/constants/xmlns.ts'; -import { getPropertyKeys } from '@getodk/common/lib/objects/structure.ts'; -import { UnknownPreloadAttributeValueNotice } from '../../error/UnknownPreloadAttributeValueNotice.ts'; +import type { PartiallyKnownString } from '@getodk/common/types/string/PartiallyKnownString.ts'; import type { BindElement } from './BindElement.ts'; -const preloadParametersByType = { - uid: [null], - date: ['today'], - timestamp: ['start', 'end'], - property: ['deviceid', 'email', 'username', 'phonenumber'], -} as const; - -const preloadParameterTypes = getPropertyKeys(preloadParametersByType); - -type PreloadParametersByType = typeof preloadParametersByType; - -type PreloadType = keyof PreloadParametersByType; - -type AssertPreloadType = (type: string) => asserts type is PreloadType; - -const assertPreloadType: AssertPreloadType = (type) => { - if (!preloadParameterTypes.includes(type as PreloadType)) { - throw new UnknownPreloadAttributeValueNotice('jr:preload', preloadParameterTypes, type); - } -}; - -const getPreloadType = (bindElement: BindElement): PreloadType | null => { - const type = bindElement.getAttributeNS(JAVAROSA_NAMESPACE_URI, 'preload'); - - if (type == null) { - return null; - } - - assertPreloadType(type); - - return type; -}; - -type PreloadParameter = PreloadParametersByType[Type][number]; - -type AssertPreloadParameter = ( - type: Type, - parameter: string | null -) => asserts parameter is PreloadParameter; +type PartiallyKnownPreloadParameter = + // eslint-disable-next-line @typescript-eslint/sort-type-constituents + PartiallyKnownString> | Extract; -const assertPreloadParameter: AssertPreloadParameter = ( - type: Type, - parameter: string | null -) => { - const parameters: ReadonlyArray> = preloadParametersByType[type]; +interface PreloadParametersByType { + readonly uid: string | null; + readonly date: PartiallyKnownPreloadParameter<'today'>; + readonly timestamp: PartiallyKnownPreloadParameter<'end' | 'start'>; - if (!parameters.includes(parameter as PreloadParameter)) { - throw new UnknownPreloadAttributeValueNotice('jr:preloadParams', parameters, parameter); - } -}; - -const getPreloadParameter = ( - bindElement: BindElement, - type: Type -): PreloadParameter => { - const parameter = bindElement.getAttributeNS(JAVAROSA_NAMESPACE_URI, 'preloadParams'); + readonly property: PartiallyKnownPreloadParameter< + // prettier-ignore + 'deviceid' | 'email' | 'phonenumber' | 'username' + >; +} - assertPreloadParameter(type, parameter); +type PreloadType = PartiallyKnownString; - return parameter; -}; +// prettier-ignore +type PreloadParameter = + Type extends keyof PreloadParametersByType + ? PreloadParametersByType[Type] + : string | null; interface PreloadInput { readonly type: Type; @@ -75,20 +35,21 @@ type AnyPreloadInput = { }[PreloadType]; const getPreloadInput = (bindElement: BindElement): AnyPreloadInput | null => { - const type = getPreloadType(bindElement); + const type = bindElement.getAttributeNS(JAVAROSA_NAMESPACE_URI, 'preload'); if (type == null) { return null; } - type Type = typeof type; - - const parameter: PreloadParameter = getPreloadParameter(bindElement, type); + const parameter: PreloadParameter = bindElement.getAttributeNS( + JAVAROSA_NAMESPACE_URI, + 'preloadParams' + ); return { type, parameter, - } satisfies PreloadInput as AnyPreloadInput; + }; }; /** @@ -118,7 +79,7 @@ export class BindPreloadDefinition implements PreloadI return null; } - return new this(input) satisfies BindPreloadDefinition as AnyBindPreloadDefinition; + return new this(input); } readonly type: Type; @@ -135,4 +96,5 @@ export type AnyBindPreloadDefinition = // eslint-disable-next-line @typescript-eslint/sort-type-constituents | BindPreloadDefinition<'uid'> | BindPreloadDefinition<'timestamp'> - | BindPreloadDefinition<'property'>; + | BindPreloadDefinition<'property'> + | BindPreloadDefinition;