[refactor to typescript] Refactor code to work on Win

raszi / node-tmp

Temporary file and directory creator for node.js

MIT License

732 stars 93 forks source link

[refactor to typescript] Refactor code to work on Win #262

Closed jbjhjm closed 3 years ago

jbjhjm commented 3 years ago

looking into node-tmp I found the in-progress typescript refactoring branch by @silkentrance and decided this would be the best starting point for my own needs. I did a little work on code to fix tests failing on windows:

PathUtils.normalize was not behaving correctly when passing an absolute Windows file path e.g. C:\stuff
PathUtils.normalize test fix: normalize uses path.sep for joining the components. The test comparision was using hardcoded fwd slashes. Updated to use path.sep too.
PathUtils.makeRelative was returning empty string if path was not relative, returns false now
This made the relative path enforcement fail before. Validation checks explicitly for false now

silkentrance commented 3 years ago

@jbjhjm Hi Jannik, great, thanks for chiming in. I did start the branch but development somehow stalled due to the pandemic and other stuff. I will have a look at this over the weekend and merge it if no issues arise.

jbjhjm commented 3 years ago

I see, do as you like @silkentrance . Tests worked fine. Right now I'm experiencing issues using the library in my windows dev environment though. In short, the file creation via fs.open works fine, but when trying to write data to it, I get permission errors. I will now try to find the reason for that.

jbjhjm commented 3 years ago

ok, less problematic than thought. Creating the file with 0o600 was the issue for me. I updated my code to use 0o755 for directories and 0o644 for files, now everything works.

silkentrance commented 3 years ago

@jbjhjm There are other issues to be dealt with as the refactoring is still in its infancy. Namely patth handling and so on. I figure once that parts of the code that have been marked as TODO/FIXME have been implemented correctly, the overall solution will work as expected.

silkentrance commented 3 years ago

@jbjhjm I am not merging the PR as it does not meet my overall quality requirements. And, given that the refactoring branch is still a WIP, there is a lot of work to be done on the branch, which is why it was not published as a PR, yet.

silkentrance commented 3 years ago

@jbjhjm However, you got me hooked again, so I will work further on the branch and make it a working PR.

jbjhjm commented 3 years ago

Alright looking forward to it @silkentrance . By now everything seems to run stable in my dev env.