Restructure
While working on adding tests to #9, I realized that the structure of the code made it a bit difficult to add tests effectively. This is hence why I've split the source into different files:
-
base: contains a base class calledUploadBasewhere each uploader will inherit from. -
browser: contains theUploadBrowseruploader (usingXMLHttpRequest) -
node: contains theUploadNode(usingform-data) -
upload: combines the two into oneUploadclass that can be used without caring which about the current environment -
function: creates theupload()function that is based on theUploadclass.
This code structure eases DX because each file is doing a specific thing.
I've also thought about moving the UploadBrowser and UploadNode into a separate folder and maybe not export them in index, but I'd need your thoughts on that
I've also thought about moving the UploadBrowser and UploadNode into a separate folder and maybe not export them in index, but I'd need your thoughts on that
This or finding a better way to do isomorphism - not sure if there's a better alternative to form-data now (maybe node's fetch supports progress now?). Would need to look more into this, the only reason why I chose form-data back then was the need for progress.
Also, I'd only aim to support node.js 18.x+. Maybe even 20.x+ if that would make things easier.
maybe node's fetch supports progress now?
I'm not sure, but I'm quite sure the browser's fetch doesn't support progress, so it's mandatory to still use XMLHttpRequest for the browser, which obviously doesn't exist in node.
Would need to look more into this, the only reason why I chose
form-databack then was the need forprogress.
I see. I use this package in the browser so I don't really know to be honest.
I'm quite sure the browser's
fetchdoesn't support progress
https://stackoverflow.com/questions/35711724/upload-progress-indicators-for-fetch
Could be done with ReadableStream from what I see, you could investigate this further if you'd like, otherwise I can look into this later this week.
That is misleading. When you pass a readable stream to fetch, you'll get notified as the steam is "read". When some data is read of a stream does not necessarily mean that it has been uploaded.
Anyway, I think the best course of action would be to review and land this, then move to other libraries/approaches later i.e. after the restructure.
Agreed, let's do that then.
TBH, I'm not sure whether maintaining the node.js logic is worthwhile anymore; seems like most uses for this would be on the browser side anyway.
Agreed, let's do that then.
Alright, I'll work on fixing CI
seems like most uses for this would be on the browser side anyway.
Agreed
TBH, I'm not sure whether maintaining the node.js logic is worthwhile anymore;
I still think keeping node support can be useful, though.