jshttp / content-disposition

Create and parse HTTP Content-Disposition header
MIT License
224 stars 43 forks source link

Feature request: browser support #22

Closed dessant closed 6 years ago

dessant commented 6 years ago

Thanks for this package! Would browser support be practical to implement? It would be great to use this library in browser extensions.

dougwilson commented 6 years ago

It may be, but I'm not familiar with browser library development and what that would entail.

dougwilson commented 6 years ago

I'm going to close this because no idea what it would take for browser support. An outline of what to do / even a pull request would likely be the next step to proceed 👍

OrangeDog commented 4 years ago

@dougwilson the only apparent issue is it's accessing global, which does not exist in browsers. A workaround is to add window.global = window.

dougwilson commented 4 years ago

Hi @OrangeDog thanks for the comment. I'm not clear on what you mean by "it's accessing global". Can you ellaborate? Our source code is https://github.com/jshttp/content-disposition/blob/master/index.js and there is no global anywhere in there I can see, at least.

OrangeDog commented 4 years ago
Uncaught ReferenceError: global is not defined
    at Object../node_modules/buffer/index.js (:4200/vendor.js:174180)
    at __webpack_require__ (:4200/runtime.js:80)
    at Object../node_modules/safe-buffer/index.js (:4200/vendor.js:258348)
    at __webpack_require__ (:4200/runtime.js:80)
    at Object../node_modules/content-disposition/index.js (:4200/vendor.js:192122)
    at __webpack_require__ (:4200/runtime.js:80)
dougwilson commented 4 years ago

Looks like the issue is within the lower-level Buffer implementation, rather than anything directly here I can change at all. This module requires the use of Buffer in order to do the encoding/decoding of parameters to provide the level of functionality that makes it useful over something like just a RegExp or a string split 👍

OrangeDog commented 4 years ago

There are certainly libraries that allow using the Buffer interface across both NodeJS and Browsers. Unfortunately it's been a while since I did that kind of thing so I can't recommend one off the top of my head. Alternatively, there is the Encoding API.

dougwilson commented 4 years ago

There are certainly libraries that allow using the Buffer interface across both NodeJS and Browsers.

My understanding is that is what WebPack is supposed to be doing for you, though. For example, your stack trace shows it is using a module named buffer, while in Node.js, there is no such module used, as that is a built-in thing. The only reason a module duplicating what is inside Node.js would presumably be to support the web browser, so it seems odd it would not work there... I would suggest opening an issue on that module to see what they say.

Alternatively, there is the Encoding API.

That is certainly a possibility, though that of course has the opposite problem: it doesn't exist in Node.js. This module's primary audience is Node.js users, though if it works in the web browser too, that is awesome. But of course suggestions would at lest need to work in Node.js for a Node.js module to try and use them :)

Note: I understand that TextDecoder and friends were added recently to Node.js, though that doesn't help this module directly, as our target is the whole of the Node.js ecosystem -- just like typically a web browser module would want to target the web of web browsers and not just, say, the latest version of just Firefox.

OrangeDog commented 4 years ago

it doesn't exist in Node.js

Yes it does. Not sure when it was added though.

dougwilson commented 4 years ago

Yes it does. Not sure when it was added though.

Yes, I included that in the note at the bottom of my comment :)

OrangeDog commented 4 years ago

I checked my notifications too quickly :)

dougwilson commented 4 years ago

But yea, my understanding is that WebPack is supposed to be providing you with web browser-compatible shims for things like Node.js buffer, though. For example, your stack trace shows it is using a module named buffer, while in Node.js, there is no such module used, as that is a built-in thing. The only reason a module duplicating what is inside Node.js would presumably be to support the web browser, so it seems odd it would not work there... I would suggest opening an issue on that module to see what they say.

dougwilson commented 4 years ago

I assume that buffer module WebPack is using is https://www.npmjs.com/package/buffer, which definately says it is made to run in the web browser... Perhaps there was an accident in there that introduced global access recently 🤔 A Ctrl+F over their GitHub source code doesn't show a global either, unfortunately, and the stack trace line numbers do not seem to match the actual source code, so not sure what is going on, exactly. Checking the node_modules/buffer/package.json file should help identify if it is this package in use there with WebPack and what version it is, too (maybe it's old and was a fixed issue or something 🤷‍♂️ ).