-
-
Notifications
You must be signed in to change notification settings - Fork 16.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
utils: use external pathIsAbsolute #2620
Conversation
Module is here: https://github.com/sindresorhus/path-is-absolute |
if (':' == path[1] && '\\' == path[2]) return true; | ||
if ('\\\\' == path.substring(0, 2)) return true; // Microsoft Azure absolute path | ||
}; | ||
exports.isAbsolute = pathIsAbsolute; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into this, it's unfortunate that since path-is-absolute
is actually exporting a platform-specific absolute function, this check is not actually backwards-compatible. Yes, path-is-absolute
is doing the "better thing", but I fear what this could break. The problem really comes down to Windows: if people are giving the path '/some/layout/index.hbs'
, our views currently thinks it's absolute and we do not prepend the root
, and so on Windows, it'll map to 'C:\\some\\layout\\index.hbs'
, where the drive is dependent on process.cwd()
.
Thoughts?
We should definately move exclusively to this module in 5.x, but in 4.x, we can probably just do return pathIsAbsolute.posix(path) || pathIsAbsolute.win32(path)
.
Ok, this has been integrated into the 5.x branch and published as part of the 5.0.0-alpha.2 release! |
Just curious, is there a reason why this lib doesn't use the native path.isAbsolute? Looking at |
Seems like it was only intended to bring |
@dougwilson thoughts? I removed the tests since it does per-platform checking.