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

Consider a more future-proof return type for measureConversion #85

Open
apasel422 opened this issue Feb 5, 2025 · 1 comment · May be fixed by #87
Open

Consider a more future-proof return type for measureConversion #85

apasel422 opened this issue Feb 5, 2025 · 1 comment · May be fixed by #87
Assignees

Comments

@apasel422
Copy link
Collaborator

apasel422 commented Feb 5, 2025

Today measureConversion is defined as follows:

[SecureContext, Exposed=Window]
partial interface PrivateAttribution {
  [Throws] Promise<Uint8Array> measureConversion(PrivateAttributionConversionOptions options);
};

The Uint8Array from the resolved promise is the encrypted report. However, it may be beneficial to future-proof the return type here by introducing a wrapper dictionary. For example, if some unencrypted structured metadata were produced by measureConversion, it would be easier to introduce it backwards-compatibly if the definition were instead something like:

// exact naming of type and field TBD
dictionary MeasureConversionResult {
  required Uint8Array report;
};

[SecureContext, Exposed=Window]
partial interface PrivateAttribution {
  [Throws] Promise<MeasureConversionResult> measureConversion(PrivateAttributionConversionOptions options);
};

This sort of wrapper could also be useful if the API gains additional methods that operate on MeasureConversionResults in their entirety.

@martinthomson
Copy link
Member

Jumping straight to the attractive nuisance... I would say "ConversionResult" would be fine. Still, it's a contested namespace, so maybe we need a prefix. It's not like the name needs to be used, so maybe "PrivateAttributionConversionResult" is fine.

As far as the shape thing goes, I'm supportive of the change. It's a bit more work to do, but I can see how it might be useful to communicate metadata alongside the report. For instance, it might be sensible for us to include the complete set of options that were used in the creation of the report, particularly when they aren't just a direct copy from the provided options (for instance, if there are UA-specific defaults, rather than defaults that we can fix in the spec).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants