refactor: use simplified basename function and remove dependency on node:path#68
refactor: use simplified basename function and remove dependency on node:path#68Phillip9587 wants to merge 1 commit intojshttp:masterfrom
basename function and remove dependency on node:path#68Conversation
|
While I understand the link to the discussion there, there are very popular polyfills for the I am not sure this actually wins us anything. |
|
@wesleytodd I understand your point. I thought about proposing |
|
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!
Just to clarify,
Totally -
Absolutely valid concern. That said, since we’re only using |
|
Not sure what it's worth, but just got here from Google because my browser app is showing a warning: I wish this pull request had been merged. Without it, now I have to go find either a different package, or spend hours trying to configure a complex hoisted build pipeline to add the necessary polyfills. This pull request would certainly make things easier for anyone using it in Vue/Angular/React/Svelte/etc. |
07a62fd to
c633189
Compare
c633189 to
bc1eae2
Compare
bc1eae2 to
a5d433a
Compare
a5d433a to
9249c56
Compare
This PR introduces a lightweight
basenamefunction to remove the dependency onnode: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: expressjs/discussions#331