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

[Bug?]: pnp mode breaks async writeFile (fs/promises) interface contract #6580

Open
1 task done
llowrey opened this issue Oct 28, 2024 · 0 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@llowrey
Copy link

llowrey commented Oct 28, 2024

Self-service

  • I'd be willing to implement a fix

Describe the bug

In pnp mode, .pnp.cjs implements the async writeFile from fs/promises by calling the synchronous writeFile and using a callback to satisfy the promise instead of calling the actual async implementation. This is not ok because the types allowed for the data argument differ between the sync and async implementations.

Sync: data <string> | <Buffer> | <TypedArray> | <DataView>

Async: data <string> | <Buffer> | <TypedArray> | <DataView> | <AsyncIterable> | <Iterable> | <Stream>

In my case, I'm passing an array of Buffer and this results in:

TypeError: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Array

This is correct behavior when calling the sync method but not when calling the async method.

Perfectly legal, and correctly functioning, code that works fine when using node_modules breaks when running in pnp mode.

To reproduce

In a project with pnp enabled, create foo.js as:

const fs = require('fs/promises')

const foo = async () => {
  await fs.writeFile('foo.txt', [Buffer.from('hello '), Buffer.from('world\n')]);
};

foo();

Then do this to see the error:

$ yarn node foo.js
node:fs:2285
    validateStringAfterArrayBufferView(data, 'data');
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Array
...

Then do this to see it work correctly when not in pnp mode:

$ node foo.js
$ cat foo.txt
hello world

Environment

System:
OS: Linux 6.10 Fedora Linux 40 (Forty)
CPU: (64) x64 AMD Eng Sample: 100-000000054-04_32/24_N
Binaries:
Node: 20.12.2 - /tmp/xfs-ce0e8161/node
Yarn: 4.5.1 - /tmp/xfs-ce0e8161/yarn
npm: 10.2.5 - /usr/local/bin/npm

Additional context

I'm happy to submit a PR but not sure what the purpose is for having the realFs abstraction. Seems like it may be necessary to have both a realFs and a realFsPromises. A little initial guidance could help me get started with a fix.

@llowrey llowrey added the bug Something isn't working label Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant