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

Remove crypto polyfill #4095

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Conversation

tillprochaska
Copy link
Contributor

The FtM TS library relies on the Node.js crypto module (which isn’t available in browsers) in order to implement namespace signatures. The crypto-browserify polyfill is quite big and there were a few questions around continued maintenance of that project, so it would be good to remove it as an dependency.

I initially planned to update the namespacing implementation in the TS lib with the Web Crypto API which is available Node.js and browsers, but this would require breaking changes to the TS API as the Web Crypto API is async.

However, as far as I can see we do not actually rely on the TS implementation of namespaces in the UI, so we should be able to drop the polyfill without any further changes to the TS lib.

@tillprochaska tillprochaska force-pushed the chore/remove-crypto-polyfill branch from 56c001e to 7531337 Compare January 8, 2025 14:46
@tillprochaska
Copy link
Contributor Author

Note to self: I was probably incorrect in that we do not use the namespace feature, seems like we do, at least here: https://github.com/alephdata/aleph/blob/develop/ui/src/react-ftm/components/common/EntityManager.ts#L68

Open question is whether it’s necessary as the backend (re-)applies namespaces anyway.

The FtM TS library relies on the Node.js `crypto` module (which isn’t available in browsers) in order to implement [namespace signatures](https://github.com/alephdata/followthemoney/blob/main/js/src/namespace.ts). The `crypto-browserify` polyfill is quite big and there were a few questions around continued maintenance of that project, so it would be good to remove it as an dependency.

I initially planned to update the namespacing implementation in the TS lib with the Web Crypto API which is available Node.js and browsers, but this would require breaking changes to the TS API as the Web Crypto API is async.

However, as far as I can see we do not actually rely on the TS implementation of namespaces in the UI, so we should be able to drop the polyfill without any further changes to the TS lib.
@tillprochaska tillprochaska force-pushed the chore/remove-crypto-polyfill branch from 7531337 to 62ba6c3 Compare January 8, 2025 14:54
@tillprochaska
Copy link
Contributor Author

Confirmed that creating entities in the table editor, network diagrams, and lists errors if the crypto module is not available.

@tillprochaska
Copy link
Contributor Author

For future reference: This is a problem at least in network diagrams. Here’s what happens when you create a new entity in a network diagram:

  1. Request POST /api/2/entitysets/<id>/entities to create the entity
  2. Request POST /api/2/entitysets/<id> to update the layout to include the position at which the new entity was created.

The layout references the namespaced entity ID. However, we do not compute the namespaced ID on the client anymore, only the raw ID.

I see multiple possibilities to solve this (but don’t have time to pursue either of them atm).

  1. Await the response of the first request before sending the second request. Couldn’t figure that out quickly right now, as there are quite a few levels of abstractions1 that mostly assume that we can get the namespaced ID synchronously.
  2. Replace the use of the crypto Node.js module in the TS lib with the Web Crypto API as I planned to do initially. However, this also means that getting the namespaced ID is async (as the Web Crypto API is async), i.e it also requires figuring out how to make the abstractions work properly with async operations.
  3. Refactor to remove the EntityManager, don’t keep entity set items as local state anymore, and refetch entities whenever the entity set items are mutated.

Footnotes

  1. Primarily the entityEditorWrapper HOC and the EntityManager.

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 this pull request may close these issues.

1 participant