Conversation
|
||
// TODO: Also support browsers that don't have TextDecoder (e.g., Edge) | ||
const utf8Decoder = new TextDecoder('utf-8'); | ||
import { decodeUtf8 } from './Utf8Decoder'; |
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.
Pascal case decodeUtf8
to be consistent?
EDIT: On the other hand, the other imports are types, this a method...
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.
No, it’s consistent as-is already because it’s a function name, not a class name.
@danroth27 @Eilon Do you know if this kind of attribution is acceptable? 1446c95 Or is any further change (e.g. third party notices file) required? |
@SteveSandersonMS good catch, we definitely need this in a 3PN both in the repo (because this source code is distributed by this repo), and in the final MSI/VSIX/NUPKG/whatever, assuming this bit of code ends up in there somehow (e.g. compiled in, or as-is text). |
@Eilon @danroth27 Added to TPN file in 5ea04f8. I think this is what you've requested so I will merge it now, but please let me know if you think anything is still missing or wrong with the attribution. |
Yeah this looks good to me, thanks! |
* Polyfill for TextDecoder * Move UTF8 decoder logic into a separate file. Don't polyfill globally. * Workaround issue in UTF8 decoder logic * Add attribution for fast-text-encoding * Added TPN for fast-text-encoding
Continuation of #1150 by @Suchiman