feat(std/node): Add back os.tmpdir() implementation #1308
Conversation
|
|
|
Thank you for your contribution. But this doesn't seem aligned to the Node.js implementation of os.tmpdir. I'd suggest we should simply follow the node.js implementation https://github.com/nodejs/node/blob/43291b98edaa682b9fa74f95e084ce7a01c85774/lib/os.js#L173-L189 And please sign the CLA |
Done.
There should now be a corporate CLA in place. |
| return base + "\\temp"; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
For readability purposes, use else.
|
It seems that node.js strips trailing slashes (and backslashes on windows) unless the temp dir is a root ( |
Yes, sorry I meant to include that in the comment. The implementation in node removes only one trailing slash: Which seems pretty arbitrary. I can do it if we want bug for bug compatibility, but it seems better to remove all trailing slashes or none, no? |
It does seem arbitrary, we should remove all trailing slashes. |
This was lost when Dino.dir was removed. This implementation is in typescript and unlike the previous implementation, it follows what the node version does, instead of using rust's std::env::temp_dir.
|
@espindola Thank you for your contribution! |
This was lost when Dino.dir was removed. This implementation is in
typescript and should do the same thing as std::env::temp_dir was
doing in the previous implementation:
https://doc.rust-lang.org/std/env/fn.temp_dir.html
The previous implementation was in the pull request denoland/deno#4213