-
Notifications
You must be signed in to change notification settings - Fork 945
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
Make sure buffer is a DataView #3127
Make sure buffer is a DataView #3127
Conversation
This code should probably be made DRY as well then? ipywidgets/packages/base-manager/src/manager-base.ts Lines 160 to 168 in 5132234
|
packages/base/src/utils.ts
Outdated
@@ -100,18 +100,25 @@ export function reject(message: string, log: boolean) { | |||
export function put_buffers( | |||
state: Dict<BufferJSON>, | |||
buffer_paths: (string | number)[][], | |||
buffers: DataView[] | |||
buffers: any[] |
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.
any
seems overly permissive. AFAICT, this should be a union of:
DataView
ArrayBuffer
- a generic type that includes a
buffer
attribute of the correct type (ArrayBuffer
). This would catch TypedArrays.
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.
Thanks, we also need to add ArrayBufferView
in the union.
Good catch, and here too: ipywidgets/packages/base/src/widget.ts Lines 219 to 228 in 5132234
|
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.
Looks good! Thanks 👍
Thanks @davidbrochart, and thanks for the review @vidartf. |
Fixes #2588