[v18.x] Wrong path returned by fs.readdir withFileTypes=true

[H4ad]

Version

18.20.1

Platform

Linux h4ad 6.5.0-27-generic #28~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 15 10:51:06 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

fs/promise

What steps will reproduce the bug?

You can reproduce the issue by running the following commands:

$ mkdir fs-issue
$ cd fs-issue
$ touch issue.txt
$ node -e "require('fs/promises').readdir('.', { withFileTypes: true, encoding: 'utf-8', }).then(console.log)" 

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

The path should be ..

[
  Dirent {
    name: 'issue.txt',
    parentPath: '.',
    path: '.',
    [Symbol(type)]: 1
  }
]

What do you see instead?

The path is different:

[
  Dirent {
    name: 'issue.txt',
    parentPath: '.',
    path: 'issue.txt',
    [Symbol(type)]: 1
  }
]

This issue only happens on fs/promises, on readdirSync, we don't see this issue:

$ node --print "require('fs').readdirSync('.', { withFileTypes: true, encoding: 'utf-8', })"
[
  Dirent {
    name: 'issue.txt',
    parentPath: '.',
    path: '.',
    [Symbol(type)]: 1
  }
]

Also, if we include recursive, we don't see the issue:

$ node -e "require('fs/promises').readdir('.', { recursive:true, withFileTypes: true, encoding: 'utf-8', }).then(console.log)"
[
  Dirent {
    name: 'issue.txt',
    parentPath: '.',
    path: '.',
    [Symbol(type)]: 1
  }
]

Additional information

This bug was first introduced at 18.20.0, on 18.19.1 the behavior is correct. On latest version of node, 21.7.2 and 20.12.1, we don't see this issue.

This bug was found by @wraithgar at https://github.com/npm/cli/pull/7352#issuecomment-2045701032

[H4ad]

The issue was probably introduced by this change introduced by https://github.com/nodejs/node/pull/51021:

https://github.com/nodejs/node/blob/f75dc4193e3faa66d0a827400c150cedf2412700/lib/internal/fs/utils.js#L200-L205

The main PR https://github.com/nodejs/node/pull/50976 didn't include these changes.

[richardlau]

cc @aduh95

[ganialhamad]

hi guys, what's the status of this issue? I can look into it if it wasn't assigned to anyone yet

[H4ad]

@gm-al Basically waiting for a PR, feel free to work on it if you want :) You can ping me when you had something to review

[jakecastelli]

Just had a quick scan, looks like the current main does not have path property any more? was that intentional? (it was still there in v20.x, v22.x and I couldn't see it mentioned in the changelog or docs

[sonsurim]

Hello! May I try to resolve this issue?

[dario-piotrowicz]

The dirent.path field has been deprecated in https://github.com/nodejs/node/pull/51020 and removed in https://github.com/nodejs/node/pull/55548 (like @jakecastelli mentioned above)

Precisely for reasons like the one presented here I assume

So this issue is no longer applicable and should be closed

cc. @aduh95

[H4ad]

@dario-piotrowicz Thanks for the reminder, closing since this is no longer applicable.