appservice: Move to new eng package #2152
Conversation
There was a problem hiding this comment.
Drawing extra attention to this since it's more than just basic typing/lint changes.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| */ | ||
| export async function putFile(context: IActionContext, site: ParsedSite, data: string | ArrayBuffer, url: string, etag: string | undefined): Promise<string> { | ||
| const options: {} = etag ? { ['If-Match']: etag } : {}; | ||
| export async function putFile(context: IActionContext, site: ParsedSite, data: string | ArrayBuffer | Uint8Array, url: string, etag: string | undefined): Promise<string> { |
There was a problem hiding this comment.
VSCode is actually passing in Uint8Array, and below in SiteClient.vfsPutItem we can support both ArrayBuffer and Uint8Array, so the easiest non-breaking change is to include | Uint8Array.
| method: 'PUT', | ||
| url, | ||
| body: typeof data === 'string' ? data : data.toString(), | ||
| body: typeof data === 'string' ? data : new TextDecoder('utf-8').decode(data), |
There was a problem hiding this comment.
The old code worked, but somewhat by luck. VSCode's FS interfaces pass around Uint8Array, which is what we are passing into this method. At runtime, in the desktop VSCode, it's actually a Node Buffer, which implements .toString(encoding?: ...), with UTF-8 being the default encoding. So, in the past we were taking the byte array and encoding it to UTF-8 before upload.
Thing is, Uint8Array (the base type also shared by web) does not do that; it has just .toString() which writes unencoded, comma-separated Uint8s.
The better solution is to use TextDecoder to be explicit about what we're trying to do. It supports ArrayBuffer, Uint8Array, and Buffer all the same. TextDecoder is more technically correct, the best kind of correct.
No description provided.