feat: hybrid cjs and esm support #82
Conversation
ts_src/index.ts
Outdated
| @@ -26,8 +26,7 @@ function base (ALPHABET: string): base.BaseConverter { | |||
| const iFACTOR = Math.log(256) / Math.log(BASE) // log(256) / log(BASE), rounded up | |||
|
|
|||
| function encode (source: Uint8Array | number[]): string { | |||
| if (source instanceof Uint8Array) { | |||
| } else if (ArrayBuffer.isView(source)) { | |||
| if (ArrayBuffer.isView(source)) { | |||
There was a problem hiding this comment.
I agree that this was originally poorly written, but this change makes the code different.
What if source is BOTH Uint8Array AND one of the other else if?
If you are 100% confident that these conditions will never overlap in any JS runtime ever, then maybe this is OK. But I would rather just leave it as is... or at least re-write it with the same semantic meaning... like wrapping the whole if else chain with a negation of the empty if.
There was a problem hiding this comment.
Actually, I'm wondering if I should remove the first if statement (ArrayBuffer one). The function just accepts Uint8Array | number[] clearly.
There was a problem hiding this comment.
Reverted back to the original code
|
npm v6 doesn't support |
.github/workflows/main_ci.yml
Outdated
| with: | ||
| node-version: 12 | ||
| node-version: 17 |
There was a problem hiding this comment.
The commit says 18 but it's 17 (EOL)
This PR adds support for cjs and esm modules.