BREAKING(fs): throw Deno.errors.NotSupported instead of SubdirectoryMoveError in move[Sync]()

[iuioiua]

What's changed

Previously, @std/fs/move threw SubdirectoryMoveError. Now, it throws Deno.errors.NotSupported.

Motivation

This change was made to reduce the API surface area by using an appropriate error class that's already built into the Deno runtime.

Migration guide

To migrate, compare against Deno.error.NotSupported instead of SubdirectoryMoveError when error-handling move() or moveSync().

- import { move, SubdirectoryMoveError } from "@std/fs/move";
+ import { move } from "@std/fs/move";

try {
  await move("./path/to/folder/subdir", "./path/to/folder");
} catch (error) {
- if (error instanceof SubdirectoryMoveError) {
+ if (error instanceof Deno.errors.NotSupported) {
    // ...
  }
}

Related

Towards #5476

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.51%. Comparing base (42693da) to head (61589a1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5532      +/-   ##
==========================================
- Coverage   96.51%   96.51%   -0.01%     
==========================================
  Files         465      465              
  Lines       37714    37705       -9     
  Branches     5576     5576              
==========================================
- Hits        36400    36391       -9     
  Misses       1272     1272              
  Partials       42       42              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kt3k]

InvalidData sounds a bit strange for describing this situation to me.

Is this worth the breaking change?

[iuioiua]

InvalidData sounds a bit strange for describing this situation to me.

How about Deno.errors.NotSupported if src is a sub-directory of dst and Deno.errors.AlreadyExists if src equals dst?

Is this worth the breaking change?

Yes. It's one less extraneous symbol that the user has to import.

[kt3k]

It's one less extraneous symbol that the user has to import.

This doesn't sound a good reason for breaking change. There are many symbols we can remove if we optimize for fewer number of symbols.

[iuioiua]

Perhaps, we're asking the wrong question. I think the question we should be asking is: why should we have a custom SubdirectoryMoveError class if there already exists an extensive collection of built-in error classes that accomplish the same thing? This is the only custom error class in @std/fs. Why? Why not use what's in-built?

[kt3k]

Ok. I changed my mind. Let's do this change, but let's use Deno.errors.NotSupported, which seems more reasonable to me.

[iuioiua]

Great! Updated.

[kt3k]

LGTM