search

minio / minio-js

MinIO Client SDK for Javascript
https://docs.min.io/docs/javascript-client-quickstart-guide.html
Apache License 2.0
951 stars 277 forks source link

fix: Deno and cloudflare workers deploy #1265

Open riderx opened 8 months ago

riderx commented 8 months ago

As I reported here: https://github.com/minio/minio-js/issues/1261 There is some issue in Deno and also in Cloudflare workers to deploy.

I used a hack with ESM to deploy, but found out the only problem was the Babel config.

The good part is it's easy to fix, since it's already stated in package.json that the min version to use the package is Node.js 16.

The node: was introduced in Node.js 16.

Since then, it also got back ported to Node.js 14 and Node.js 12, but I followed what was set in package.json and aligned the test to run only from 16.

I also updated the lint step to run on the latest LTS, that is optional I can remove it if required.

Link: who talks about supported version: https://2ality.com/2021/12/node-protocol-imports.html

Node.js doc: https://nodejs.org/api/esm.html#node-imports

riderx commented 8 months ago

A Pr will be done also in submodules xml2js and xml to apply the same fix

riderx commented 8 months ago

I found out you where using fast-xml-parser in your test but not in the lib. This lib is compatible with all env and requires a minimum number of changes. I made the change and it also reduced the bundle size: CleanShot 2024-02-12 at 20 27 13@2x The test passed: CleanShot 2024-02-12 at 21 57 22@2x I found only one change during my test: <?xml version="1.0" encoding="UTF-8" standalone="yes"?> header is never present with the new lib. But it seems it passed your test. It should be double checked to be sure it's working without the header. If it's required, i can update the code

riderx commented 8 months ago

I also had to replace stream-json for the same reason, alternative was recommended here: https://github.com/uhop/stream-json/issues/91

riderx commented 8 months ago

Last require change: mime-types to mime package For that one, I honestly didn't want to change it since the change to the original repo was minimal: https://github.com/riderx/mime-types/commit/fdb301ed6e6468c706ffe9ffe6b19b9494df0e94 PR here: https://github.com/jshttp/mime-types/pull/120 But the package creator is against that change, since express is still supporting node 0.10: https://github.com/jshttp/mime-types/issues/115 I made the change he suggested, and put mime package.

That led me to make the big change you see in the last commit. I had to switch to type module and then the build script was totally broken. The easy way was, for me, to use a more modern builder. I hope this will not become too big of a change.

pellicceama commented 1 month ago

It would be awesome to prioritize this to make it work on Cloudflare workers.