-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
Open
Labels
discussIssues opened for discussions and feedbacks.Issues opened for discussions and feedbacks.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.Issues that need assistance from volunteers or PRs that need help to proceed.pathIssues and PRs related to the path subsystem.Issues and PRs related to the path subsystem.
Description
I believe this was introduced somewhere in #5123, which changed the behavior of the ext
argument.
This is observed on all supported branches and was even recently backported to 4.x.
Documentation:
https://nodejs.org/api/path.html#path_path_basename_path_ext
Observe the input and try to predict the output:
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'b'))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a'))
[ '', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a/'))
[ 'a', '', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'a//'))
[ 'a', 'a', '' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aa'))
[ 'a', 'a/', 'a//' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'bb'))
[ 'a', 'a', 'a' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aaa'))
[ 'a', 'a', 'a//' ]
> ['a', 'a/', 'a//'].map(x => path.posix.basename(x,'aaaa'))
[ 'a', 'a', 'a' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x))
[ 'dd', 'dd', 'dd', 'dd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'd'))
[ 'd', 'd', 'd', 'd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'dd'))
[ '', 'dd', 'dd', 'dd' ]
> ['dd', '/dd', 'd/dd', 'd/dd/'].map(x => path.posix.basename(x, 'ddd'))
[ 'dd', 'dd', 'dd', 'dd/' ]
There are more, but all the inconsistencies with the previous behavior involve at least one of those:
- Either the
path
ends with/
, - Or
ext
includes/
, - Or
ext
equals to the actual resolved basename (i.e.path.endsWith('/' + ext)
).
More specifically, the following check covers all the cases inconsistent behavior to my knowledge:
path.endsWith('/') || ext.includes('/') || path.endsWith('/' + ext)
(note that it also includes cases of consistent behavior).
Reminder: before #5123, this was how ext
behave:
if (ext && f.substr(-1 * ext.length) === ext) {
f = f.substr(0, f.length - ext.length);
}
I.e. it just sliced off the suffix (after doing everything else).
Metadata
Metadata
Assignees
Labels
discussIssues opened for discussions and feedbacks.Issues opened for discussions and feedbacks.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.Issues that need assistance from volunteers or PRs that need help to proceed.pathIssues and PRs related to the path subsystem.Issues and PRs related to the path subsystem.