Skip to content
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

Add filesystem utils #148

Merged
merged 19 commits into from
Oct 17, 2023
Merged

Add filesystem utils #148

merged 19 commits into from
Oct 17, 2023

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Oct 17, 2023

This commit adds several utility functions that are related to the filesystem in some way.

  • Most of the functions here are wrappers around basic filesystem operations with better error handling. Node's fs.promises module is somewhat difficult to work with because the errors it produces lack proper stacktraces. The functions that directly use fs.promises wrap caught errors in order to supply the missing stacktraces.
  • Two functions make it easier to read and write JSON files (with support for JSON-compatible interfaces, such as JSON5).
  • One function, createSandbox, is designed to wrap tests that need a temporary directory to work within, such as those for a command-line tool that makes changes to the filesystem.

Here are places where we currently use these utilities (or something like them):

One note about these utilities is that they require Node to use and will not work in the browser. Because we already create two kinds of bundles, one for CommonJS and another ESM, it would be difficult to create a second level of bundles, one for Node and another for the browser. Instead of requiring more complexity around the bundle configuration, this commit instead introduces another way to import the package.

By default, you'll get all exports that are guaranteed to be cross-platform. That means that the file utilities won't show up:

// ❌
import { readFile } from "@metamask/utils";

If you want all of the cross-platform exports plus the Node-specific ones, you will have to import "@metamask/utils/node". For instance:

// ✅
import { readFile } from "@metamask/utils/node";

Relates to MetaMask/module-lint#5.

@socket-security
Copy link

socket-security bot commented Oct 17, 2023

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
@types/jest-when 3.5.3 None +0 7.01 kB types
jest-when 3.6.0 None +0 24.8 kB timkindberg

@mcmire mcmire changed the title Add file utils Add filesystem utils Oct 17, 2023
@mcmire mcmire force-pushed the add-file-utils branch 2 times, most recently from db348d9 to 8b1fc00 Compare October 17, 2023 03:34
Base automatically changed from add-error-utils to main October 17, 2023 14:57
This commit adds several utility functions that are related to the
filesystem in some way.

- Most of the functions here are wrappers around basic filesystem
  operations with better error handling. Node's `fs.promises` module is
  somewhat difficult to work with because [the errors it produces lack
  proper stacktraces][1]. The functions that directly use `fs.promises`
  wrap caught errors in order to supply the missing stacktraces.
- Two functions make it easier to read and write JSON files (with support
  for JSON-compatible interfaces, such as [JSON5][2]).
- One function, `createSandbox`, is designed to wrap tests that need
  a temporary directory to work within, such as those for a command-line
  tool that makes changes to the filesystem.

Here are places where we currently use these utilities (or something
like them):

- https://github.com/MetaMask/action-utils/blob/54ddd730746668cb4c1c88b4edfa720cbecf5e32/src/file-utils.ts
- https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/src/fs.ts
- https://github.com/MetaMask/create-release-branch/blob/3556dee47163c921186051be7a1f3c98e2049db9/tests/helpers.ts

One note about these utilities is that they require Node to use and will
not work in the browser. Because we already create two kinds of bundles,
one for CommonJS and another ESM, it would be difficult to create a
second level of bundles, one for Node and another for the browser.
Instead of requiring more complexity around the bundle configuration,
this commit instead introduces another way to import the package.

By default, you'll get all exports that are guaranteed to be
cross-platform. That means that the file utilities won't show up:

``` typescript
// ❌
import { readFile } from "@metamask/utils";
```

If you want all of the cross-platform exports plus the Node-specific
ones, you will have to import "@metamask/utils/node". For instance:

``` typescript
// ✅
import { readFile } from "@metamask/utils/node";
```

[1]: nodejs/node#30944
[2]: https://www.npmjs.com/package/json5
@mcmire mcmire marked this pull request as ready for review October 17, 2023 14:58
@mcmire mcmire requested a review from a team as a code owner October 17, 2023 14:58
package.json Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Outdated Show resolved Hide resolved
src/fs.ts Show resolved Hide resolved
src/node.ts Show resolved Hide resolved
mcmire and others added 16 commits October 17, 2023 10:25
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@mcmire
Copy link
Contributor Author

mcmire commented Oct 17, 2023

@Mrtenz I believe I've addressed your feedback, do you mind checking again?

Mrtenz
Mrtenz previously approved these changes Oct 17, 2023
src/fs.test.ts Show resolved Hide resolved
@mcmire mcmire merged commit 12b5003 into main Oct 17, 2023
16 checks passed
@mcmire mcmire deleted the add-file-utils branch October 17, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants