jshttp
refactor: use simplified `basename` function and remove dependency on `node:path`
This PR introduces a lightweight basename function to remove the dependency on node:path, enabling usage in environments like the browser. Additionally, new test cases have been added to validate the function's handling of various path formats.
Ref: https://github.com/expressjs/discussions/issues/331
While I understand the link to the discussion there, there are very popular polyfills for the path module for use in the browser, for example with webpack: https://v4.webpack.js.org/configuration/node/
I am not sure this actually wins us anything.
@wesleytodd I understand your point. I thought about proposing pathe which is used for example by Nuxt or unenv but content-dispositions use case is so simple and only uses the basename function. Merging this PR would allow users to use this package for exampel with cloudflare's workerd runtime.
I think it is even supported there: https://developers.cloudflare.com/workers/runtime-apis/nodejs/path/
This one is one of the most ubiquitously supported ones because EVERYTHING imports it lol.
I'm neutral on this change. I wonder if this will break anything, given that several operating systems need to be considered, which is what Node would do for us
Thanks for the feedback!
I think it is even supported there: https://developers.cloudflare.com/workers/runtime-apis/nodejs/path/
Just to clarify, node:path is only available in Cloudflare’s Workerd runtime when the nodejs_compat or nodejs_compat_v2 flags are enabled. But that’s just one example - other runtimes may have different levels of support. For example, Vercel’s Edge Runtime does not support node:path: https://vercel.com/docs/functions/runtimes/edge#compatible-node.js-modules
This one is one of the most ubiquitously supported ones because EVERYTHING imports it lol.
Totally - path is everywhere. But we also need to consider browser compatibility. In environments like bundlers or edge runtimes targeting broader platforms, relying on path can lead to unnecessary overhead (due to full-module polyfills) or breakage unless polyfills are explicitly configured. Since content-disposition only uses basename, a lightweight replacement helps avoid these issues and makes the package more flexible across environments.
I'm neutral on this change. I wonder if this will break anything, given that several operating systems need to be considered, which is what Node would do for us.
Absolutely valid concern. That said, since we’re only using basename to strip everything before the last slash or backslash (to get the filename), the logic stays simple and OS-agnostic. I’ve added extensive tests to cover edge cases across platforms—if there’s anything I missed, I’m happy to add it.