feat(dependencies): Add unit tests checking for circular dependencies and orphan modules

[kalisjoshua]

Is there any interest in including a unit test for auditing the repository for circular dependency chains or orphan files? I added two files to illustrate the circular dependency feature; they aren't intended to stay (hopefully obvious).

There seem to be a lot (42) of files that aren't used as imports anywhere. Are these used somehow that isn't immediately obvious?

[
    "/Users/boat/Documents/deno/deno_std/crypto/_util.ts",
    "/Users/boat/Documents/deno/deno_std/crypto/_benches/bench.ts",
    "/Users/boat/Documents/deno/deno_std/dotenv/testdata/app_load.ts",
    "/Users/boat/Documents/deno/deno_std/dotenv/testdata/app_with_restricted_env_access.ts",
    "/Users/boat/Documents/deno/deno_std/dotenv/testdata/app_with_restricted_env_access_sync.ts",
    "/Users/boat/Documents/deno/deno_std/dotenv/testdata/app_defaults.ts",
    "/Users/boat/Documents/deno/deno_std/dotenv/testdata/app_load_parent.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/check_deprecation.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/release/01_bump_version.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/release/03_release.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/release/02_create_pr.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/check_browser_compat.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/check_licence.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/check_assertions.ts",
    "/Users/boat/Documents/deno/deno_std/_tools/check_doc_imports.ts",
    "/Users/boat/Documents/deno/deno_std/testing/fast_check_example.ts",
    "/Users/boat/Documents/deno/deno_std/testing/chai_example.ts",
    "/Users/boat/Documents/deno/deno_std/testing/sinon_example.ts",
    "/Users/boat/Documents/deno/deno_std/http/testdata/file_server_as_library.ts",
    "/Users/boat/Documents/deno/deno_std/http/testdata/simple_server.ts",
    "/Users/boat/Documents/deno/deno_std/http/testdata/simple_https_server.ts",
    "/Users/boat/Documents/deno/deno_std/http/bench.ts",
    "/Users/boat/Documents/deno/deno_std/examples/catj.ts",
    "/Users/boat/Documents/deno/deno_std/examples/welcome.ts",
    "/Users/boat/Documents/deno/deno_std/examples/chat/server.ts",
    "/Users/boat/Documents/deno/deno_std/examples/gist.ts",
    "/Users/boat/Documents/deno/deno_std/examples/echo_server.ts",
    "/Users/boat/Documents/deno/deno_std/examples/cat.ts",
    "/Users/boat/Documents/deno/deno_std/examples/flags.ts",
    "/Users/boat/Documents/deno/deno_std/examples/curl.ts",
    "/Users/boat/Documents/deno/deno_std/yaml/example/inout.ts",
    "/Users/boat/Documents/deno/deno_std/yaml/example/parse.ts",
    "/Users/boat/Documents/deno/deno_std/yaml/example/sample_document.ts",
    "/Users/boat/Documents/deno/deno_std/yaml/example/dump.ts",
    "/Users/boat/Documents/deno/deno_std/wasi/snapshot_preview1_test_runner.ts",
    "/Users/boat/Documents/deno/deno_std/fs/testdata/empty_dir.ts",
    "/Users/boat/Documents/deno/deno_std/fs/testdata/exists_sync.ts",
    "/Users/boat/Documents/deno/deno_std/fs/testdata/0.ts",
    "/Users/boat/Documents/deno/deno_std/fs/testdata/empty_dir_sync.ts",
    "/Users/boat/Documents/deno/deno_std/fs/testdata/exists.ts",
    "/Users/boat/Documents/deno/deno_std/console/_tools/generate_data.ts",
    "/Users/boat/Documents/deno/deno_std/console/_tools/compare_with_rust.ts",
]

Are these features handled somewhere else? If so, no worries. I used this as a chance to learn a little more about the Deno ecosystem and publish my first module. I'm open to suggestions or improvements; including closing this PR for any reason.

[kt3k]

There seem to be a lot (42) of files that aren't used as imports anywhere. Are these used somehow that isn't immediately obvious?

They seem mostly used.

• Files under _tools - scripts for some tasks, used for development.
• Files with bench in its path - Benchmark scripts
• Files with testdata in its path - Usually used from test cases.
• Files with example in its path - Examples
• wasi/snapshot_preview1_test_runner.ts - wasi preview1 entrypoint

Only crypto/_util.ts looks dead code to me.

We used to check circular dependency only in std/node to make sure that it can be bundled with esbuild, but now we removed that check when we migrated std/node to Deno main repository. In my view, circular dependencies are just fine unless there's special reason to avoid it.

[kalisjoshua]

The original idea for this sort of thing occurred to me when I was reading through the style guide and specifically the mention of circular imports.

@kt3k would it be helpful to keep a check like this with specific files/patterns excluded from the results to be explicit about what exceptions are being made and the opportunity to document them and why?

I've updated the PR with additional patterns to "skip" using walk to exclude the files you mentioned. There are still 2 exceptions, though I expect one dotenv/load.ts to be used somewhere still.

Just asking the questions to have to conversation.

[kalisjoshua]

I didn't mean to close the PR; bad muscle memory.

[iuioiua]

In my view, circular dependencies are just fine unless there's special reason to avoid it

+1

@kalisjoshua, is there a particular issue you're experiencing due to circular dependencies? They're usually not problematic.

[kalisjoshua]

No issue @iuioiua I was just trying to create awareness about a rule I saw in the style guide that I didn't see being enforced. I thought it might be helpful to have something checking something like these issues because it is hard for humans to do so reliably.

If this isn't desired or needed that's fine but it might be a good idea to remove it from the style guide as a rule.

[iuioiua]

I see. I think the solution here is modifying or removing the line in the style guide, as it suggests circular imports are problematic:

In particular, be careful not to introduce circular imports.

As Yoshiya said, circular imports needed to be avoided when the Node compatibility layer was part of the Standard Library. But this is no longer the case.

[kt3k]

dotenv/load.ts is a part of public API of dotenv module (it automatically loads .env as side effect).

How does report detect orphan files? The algorithm doesn't look aligned to the rule of std. The public APIs don't necessarily be exported from mod.ts in std. (For example, testing and fmt don't have mod.ts)

[kt3k]

Closing without merge as there seem no clear path forward. Thanks for your suggestion anyway