Modernize window.crypto access constants

matrix-org / matrix-js-sdk

Matrix Client-Server SDK for JavaScript

Apache License 2.0

1.49k stars 578 forks source link

Modernize window.crypto access constants #4169

Closed turt2live closed 2 months ago

turt2live commented 2 months ago

For https://github.com/element-hq/element-web/pull/27326

Checklist

[ ] Tests written for new code (and old code if feasible).
[x] New or updated public/exported symbols have accurate TSDoc documentation.
[x] Linter and other CI checks pass.
[ ] Sign-off given on the changes (see CONTRIBUTING.md).

turt2live commented 2 months ago

I believe the Element Web build error is failed (or lack of) branch matching to the react-sdk.

turt2live commented 2 months ago

Sorry for the spam here. As is hopefully obvious from the commit history, I tried to use @types/serviceworker and that promptly exploded in my face.

My vote is to give up for now and put a help wanted label on it.

richvdh commented 2 months ago

My vote is to give up for now and put a help wanted label on it.

I don't really understand. If you're not going to keep working on the PR, could you close it please?

turt2live commented 2 months ago

I'm not sure I understand the action requested here. Should the crypto indirection be removed?

As for closing the PR: I'm saying we give up on trying to get types to work, but the PR is required to work. I cannot commit to debugging the entire type system much further, so a help wanted issue feels like the correct approach.

richvdh commented 2 months ago

I'm not sure I understand the action requested here. Should the crypto indirection be removed?

I'm suggesting that, rather than doing anything special for service workers, you change lines 19-21 to be:

export let crypto = globalThis.crypto;
export let subtleCrypto = crypto?.subtle ?? crypto?.webkitSubtle;  // or just `crypto?.subtle`
export let TextEncoder = globalThis.TextEncoder;

... and be done.

As for closing the PR: I'm saying we give up on trying to get types to work, but the PR is required to work. I cannot commit to debugging the entire type system much further, so a help wanted issue feels like the correct approach.

oh right. Well, that was far from obvious from your comment :)