fs-extra: Define missing fs.realpath.native

[Andarist]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change.
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm test <package to test>.

If changing an existing definition:

• Provide a URL to documentation or source code which provides context for the suggested changes: <>
• If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

---

Based on:
https://github.com/jprichardson/node-fs-extra/blob/1625838cdfc65a1bbf28ab5fa962a75805629b9c/lib/fs/index.js#L127-L129
and

DefinitelyTyped/types/node/fs.d.ts

Lines 797 to 804 in dcc9ee4

	function native(
	path: PathLike,
	options: BaseEncodingOptions | BufferEncoding | undefined | null,
	callback: (err: NodeJS.ErrnoException | null, resolvedPath: string) => void
	): void;
	function native(path: PathLike, options: BufferEncodingOption, callback: (err: NodeJS.ErrnoException | null, resolvedPath: Buffer) => void): void;
	function native(path: PathLike, options: BaseEncodingOptions | string | undefined | null, callback: (err: NodeJS.ErrnoException | null, resolvedPath: string | Buffer) => void): void;
	function native(path: PathLike, callback: (err: NodeJS.ErrnoException | null, resolvedPath: string) => void): void;

This exists since fs-extra@8.1.0:
jprichardson/node-fs-extra@fa661f3

[typescript-bot]

@Andarist Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

• fs-extra on npm, on unpkg
  • owner-approval: @peterblazejewicz

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ❌ Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 7 days.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 51155,
  "author": "Andarist",
  "headCommitOid": "04dff5135b2d7a2ed6415e86514ed28fb6306280",
  "lastPushDate": "2021-02-21T13:33:07.000Z",
  "lastActivityDate": "2021-02-21T15:55:19.000Z",
  "maintainerBlessed": false,
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "fs-extra",
      "kind": "edit",
      "files": [
        {
          "path": "types/fs-extra/fs-extra-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/fs-extra/index.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "alan-agius4",
        "midknight41",
        "shiftkey",
        "mees-",
        "jrockwood",
        "sangdth",
        "ffflorian",
        "peterblazejewicz",
        "NotWoods"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2021-02-21T15:55:19.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "orta",
      "date": "2021-02-18T16:30:12.000Z",
      "abbrOid": "b12c36c"
    }
  ],
  "ciResult": "pass"
}

[typescript-bot]

🔔 @alan-agius4 @midknight41 @shiftkey @mees- @jrockwood @sangdth @ffflorian @peterblazejewicz @NotWoods — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[typescript-bot]

@Andarist The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[Andarist]

as this project is related to @types/node I believe it's better for both to keep a similar shape rather than to optimize for unified-signatures

[peterblazejewicz]

Yes, that's correct, but it is also better to use inline exclusion, not general rule, so per-readme for DT:
https://github.com/DefinitelyTyped/DefinitelyTyped#linter-tslintjson
@types/nodes are just so complex, it would be unrealistic to exclude via inline comments,
also we could probably just do something like (without modifying tslint):

export namespace realpath {
    const native: {
        (path: PathLike, options?: fs.BaseEncodingOptions | BufferEncoding | null): Promise<string>;
        (path: PathLike, options: fs.BaseEncodingOptions | string | undefined | null): Promise<string | Buffer>;
        (path: PathLike, options: fs.BufferEncodingOption): Promise<Buffer>;
    } & typeof fs.realpath.native;

(the expected return types would be different and there is no inferred return type). There would be no need to retype fs version (dry)

[orta]

I think this feedback makes sense, punting on merging to give discussion a bit more time

[Andarist]

Adjusted as proposed 👍

[typescript-bot]

👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings.

Let’s review the numbers, shall we?

Comparison details 📊

	master	#51155	diff
Batch compilation
Memory usage (MiB)	73.5	69.3	-5.6%
Type count	9681	9700	0%
Assignability cache size	1853	1853	0%
Language service
Samples taken	688	699	+2%
Identifiers in tests	688	699	+2%
getCompletionsAtPosition
Mean duration (ms)	371.1	373.7	+0.7%
Mean CV	9.1%	9.6%
Worst duration (ms)	503.6	478.3	-5.0%
Worst identifier	Buffer	modeStr
getQuickInfoAtPosition
Mean duration (ms)	371.6	374.0	+0.7%
Mean CV	8.4%	8.2%
Worst duration (ms)	469.6	479.1	+2.0%
Worst identifier	then	buffer

It looks like nothing changed too much. I won’t post performance data again unless it gets worse.

[typescript-bot]

Re-ping @alan-agius4, @midknight41, @shiftkey, @mees-, @jrockwood, @sangdth, @ffflorian, @peterblazejewicz, @NotWoods:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

[typescript-bot]

@Andarist The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[peterblazejewicz]

LGTM!
@Andarist thx for adjusting the PR and accepting proposed changes to your original ideas. kudos

[typescript-bot]

@orta Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[typescript-bot]

I just published @types/fs-extra@9.0.8 to npm.

[idan-at]

Hey,

FYI, some types are not recognized when using @types/node for node 12.

Here are the errors:

node_modules/@types/fs-extra/index.d.ts(232,38): error TS2694: Namespace '"fs"' has no exported member 'BaseEncodingOptions'.
node_modules/@types/fs-extra/index.d.ts(233,38): error TS2694: Namespace '"fs"' has no exported member 'BufferEncodingOption'.
node_modules/@types/fs-extra/index.d.ts(234,38): error TS2694: Namespace '"fs"' has no exported member 'BaseEncodingOptions'.

Those are exposed here, in the most recent types for node (14?), but not here for node 12, nor here for node 13.

Is there anything that I can do to fix those errors?

[peterblazejewicz]

Is there anything that I can do to fix those errors?

@idan-at IMO we can extract those interfaces to inline version with fs-extra, would that fix the problem? TS does not care about the sources, just about the shape of the type.
Those inline dependencies are easy to miss in context of backward compatibility without using package.json deps, could be easily overlooked, so thx for bringing this up,

[idan-at]

You mean copy them from the node types? That can work, I guess. tho this can be problematic, for example if a bug will discover within those types - node types will be fixed, but not the inline ones.

From my end - I think I'll just pin to a specific version until I migrate to node14. I think it's the easiest solution

[dscalzi]

Node 12 is still LTS, so this needs to be fixed. Version has to be pinned to "@types/fs-extra": "9.0.7" until then.. Personally I'm unable to upgrade until frameworks like Angular/Electron are updated to support Node 14. I'm sure a lot of people are in the same boat.

[ceisele-r]

For reference, this is the issue for the compilebreak this causes in node 12: #51521

[forivall]

I just submitted a fix after this caused plenty of CI issues >_< #51677