feat(stdlib): Add user-friendly file system module#1966
Conversation
spotandjake
left a comment
There was a problem hiding this comment.
The code looks good to me, Most of my comments were related to graindoc.
Two other things:
- Currently the
sys/path is used for more low levelwasiinteractions. I think we should either move these wasi interactions elsewhere or put this module elsewhere. - Are we currently able to add some better streaming functions such as
File.open,File.readLineandFile.close?
be13437 to
c66b46b
Compare
ospencer
left a comment
There was a problem hiding this comment.
Overall, I love the API design! My major piece of feedback here is that I don't think this module belongs in the sys subdirectory, but in the regular stdlib folder. The sys folder was really just meant for the low-level wasi stuff (and probably should have been named just wasi).
I envision that moving forward to Preview 2, the wasi folder will just have the generated bindings to each of the wasi standards, and we'll have high-level libraries in the regular standard library. We can note in the module doc that use of that library incurs a dependency on a particular wasi world. For now, we can say that use of the module requires WASI Preview 1.
marcusroberts
left a comment
There was a problem hiding this comment.
I read all the way through and didn't spot anything, looks like a nice usable interface too.
Thoughts on maybe in 0.6 we rename the |
We did make that rename happen. I do see a world in which it makes sense to still have this separated, though in environments where you're not allowed a filesystem or otherwise, WASI makes it pretty easy to virtualize. |
|
@alex-snezhko Do you mind rebasing this? |
|
@ospencer rebased |
spotandjake
left a comment
There was a problem hiding this comment.
Everything looks great, I've been using this library for months locally and haven't run into a single problem.
Should we provide a Utf8.readLine? in cases with big files having a streamed api seems helpful, maybe this is a future addition when we have scoped resouces so the user doesn't need to worry about File.open and File.close.
|
I was thinking we could build out a more comprehensive streaming API in the future, I feel like there are a few different directions we could go |
| * | ||
| * @since v0.6.5 | ||
| */ | ||
| provide let makeSymlink = (linkContents, targetBaseDirPath=None, targetPath) => { |
There was a problem hiding this comment.
I think this should be called createSymlink.
There was a problem hiding this comment.
Ok going to rename makeDir to createDir as well for consistency
ac1fd34 to
600b4ad
Compare
|
Is this currently being blocked by anything? |
ospencer
left a comment
There was a problem hiding this comment.
This looks good to go! @alex-snezhko Can you update the @since versions and then merge?
600b4ad to
73a217b
Compare
|
Aren't the versions already up-to-date @ospencer ? |

Add a file system module that is more high-level and user-friendly than the current
sys/fileCloses #211