Open blakehurd opened 2 weeks ago
Wow, thanks, I appreciate all these detailed proposals!
re 1:
The JS thread was halting when coming across the undefined references to GM API 3
Instead of removing GM_xmlhttpRequest
completely, I'd like to know the exact reason why this happens. The way I see it, this code should prevent the script from failing if GM_xmlhttpRequest
is not defined:
var _GM_xmlhttpRequest = /* @__PURE__ */ (() => typeof GM_xmlhttpRequest != "undefined" ? GM_xmlhttpRequest : void 0)();
Unless GM_xmlhttpRequest
is aliased to another thing in GM API 4. In that case I think switching the order here is enough: https://github.com/Sec-ant/gm-fetch/blob/f59b5e450ebcf3b25f78bdf5ac6f71a5cba2a323/src/index.ts#L24
re 2:
I prefer to keep the abort mechanism for script handlers that support it. But yeah this one is tricky, it seems different script handlers have different behaviors: https://github.com/trim21/gm-fetch/issues/56#issuecomment-1492859094
re 3:
I unified all kinds of request payload into a blob and use binary mode to send them because I believe there were some issues about formdata payload when I developed this script. Deleting binary: true
can break many things. Maybe now it's time to revisit current options. https://github.com/Sec-ant/gm-fetch/blob/f59b5e450ebcf3b25f78bdf5ac6f71a5cba2a323/src/index.ts#L37-L42
Anyway, these problems are tougher than I thought. Thanks for bringing up all the problems you encountered in Greasemonkey. I don't think I'll accept a PR to fix them the way you did, but I think I can take a further look into them later when I have time.
Per your invitation on https://github.com/lisonge/vite-plugin-monkey/issues/166 documenting an issue and will look to put together a PR for it, based on your preference(s) here.
There are a couple of options here, so figured a ticket would be best place to discuss before putting together a PR. In the meantime, I forked the code to make these modifications, so I'm not blocked on this issue. Having this implementation worked out great otherwise, so thank you!
For context: I was using this library + vite-plugin-monkey, yet with JS instead of TS. The resulting userscript worked on Tampermonkey but not on Greasemonkey. So the issues described and following recommendations are aimed at getting where my local fork got to -- a solution that seems to work on both. If these changes are made (or satisfactory alternatives), then I should be able to switch from a local fork back to pulling this library from npm directly.
Recommendation: remove support for GM_xmlhttpRequest and require use of GM.xmlHttpRequest. Tampermonkey supports GM API 4 just fine, so I don't see a particular reason to support GM API 3 anymore. API 4 has existed for ~7 years now.
Recommendation: remove use of { abort } = on the xhr result and delete references to the variable. You could branch the logic based on GM.info.scriptHandler if it were important to support both, but for my purposes I didn't need abortable support.
Recommendation: remove use of binary flag or support taking in a parameter that activates it.