-
Notifications
You must be signed in to change notification settings - Fork 444
feat(decoders): allow preloadinng #2431
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
base: main
Are you sure you want to change the base?
Conversation
| /** | ||
| * Available decoder names for preloading | ||
| */ | ||
| export type DecoderName = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make this enum and take it outside?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a general "Decoders" class, and have it own a Map of decoder name to decoder configuration.
Add a "registerDecoder" to add a new one
Add configurations for all 6 of these here
Add an "initialize" method for creating a new instance of the decoder, which will get stored in the Map itself. That instance will have a function that does that decode on a standard set of attributes.
The standard/default configuration should include the WASM path to load from (at least as an over-ride).
It should also include a flag as to whether to pre-load that item
When calling preload, ONLY the items flagged for preloading will get preloaded.
Calling load on an item will return the previously created async wait function, OR will create one as required. That is, it has to be thread safe for preloading starting multiple times and not load the WASM multiple times.
As part of the configuration for each one, have the WASM path and the
| * - string[]: array of decoder names to preload (e.g., ['htj2k', 'jpeg2000', 'jpegls']) | ||
| * - false/undefined: no preloading (default) | ||
| */ | ||
| preloadDecoders?: boolean | string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest making this a configuration object for the loaders that UPDATES the existing loader configuration, on a per item basis, maybe with an overall flag to preload everything. That way a completely new decoder can be added just by configuring it here. Also allows specfying the wasm path and other required custom attributes depending on configuration.
Context
Allows preloading decoders, default -> no network request for all. configured to preload -> network request for all, configured with array of names of decoders -> only network requests for those.