search

unjs / unenv

🕊️ Convert javaScript code to be runtime agnostic
MIT License
464 stars 22 forks source link

refactor(node/buffer): update cloudflare hybrid exports #321

Closed anonrig closed 1 month ago

anonrig commented 1 month ago

Let's reflect the truth for cloudflare buffer hybrid module. Please validate before landing this.

pi0 commented 1 month ago

Thanks for PR. But with this, if there is a code that imports node:buffer, it will loose access to the same exports from Node.js.

$ Object.keys(require('buffer'))
[
  'Buffer',            'SlowBuffer',
  'transcode',         'isUtf8',
  'isAscii',           'kMaxLength',
  'kStringMaxLength',  'btoa',
  'atob',              'constants',
  'INSPECT_MAX_BYTES', 'Blob',
  'resolveObjectURL',  'File'
]

Can you please clarify the goal of this?

anonrig commented 1 month ago

I'm trying to get myself familiar with unenv repository :) I've updated the pull-request.

anonrig commented 1 month ago

Could you please add a test (i.e. asserting the types of the moved exports).

Any suggestions on where and how to test them?

vicb commented 1 month ago

Any suggestions on where and how to test them?

My suggestion was to check #311 -> https://github.com/unjs/unenv/blob/main/test/workerd/tests.mjs

anonrig commented 1 month ago

@vicb Would you mind re-reviewing this?