piotrwitek / ts-mocha

Mocha thin wrapper that allows running TypeScript tests with TypeScript runtime (ts-node) to get rid of compilation complexity
MIT License
190 stars 25 forks source link

Bump tsconfig-paths and place in peerDependencies #82

Open owenallenaz opened 1 year ago

owenallenaz commented 1 year ago

When running unit tests in my application it takes over 3 minutes to begin running the tests because of a complex interaction between the dated version of tsconfig-paths requested by ts-mocha and our react and material-ui heavy stack. The older version of tsconfig-paths has a very inefficient loading mechanism that results in hundreds of thousands of additional file system reads similar to this, taken using strace:

[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.js", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000035>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.json", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000034>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.node", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000035>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.ts", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000035>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.tsx", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000107>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.css", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000088>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.scss", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000086>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.sass", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000073>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.pcss", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000083>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.stylus", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000071>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.styl", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.003708>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.less", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000071>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.sss", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000065>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.gif", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000071>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.jpeg", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000076>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.jpg", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000197>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.png", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000100>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.svg", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000111>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.mp4", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000069>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.webm", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000077>
[pid  2047] statx(AT_FDCWD, "/app/@mui/utils/index.ogv", AT_STATX_SYNC_AS_STAT, STATX_ALL, 0x7ffddce5bb50) = -1 ENOENT (No such file or directory) <0.000115>

For every file tsconfig-paths loads, it's attempting to read it with 21 different extensions multiplied by the total number of files in the project. In my project this is 182,000 file reads. When I update to a newer version of tsconfig-paths this changes to 3,000 file reads. This is due to another package increasing the possible extensions in require.extensions. Looking at your readme it seems like your intent was to make this an optional dependency, such that if a user wants it, they should include it. In this case I believe the proper mechanism is actually a peerDependency with it being marked as optional via peerDependenciesMeta (https://docs.npmjs.com/cli/v9/configuring-npm/package-json#peerdependenciesmeta). This means it will not be downloaded unless the user adds it to their package.json, and then it will use whatever version they download but will warn if they aren't using ^4.1.1.

piotrwitek commented 1 year ago

Hey @owenallenaz, thanks for this PR. Indeed that was my intention but back in the day "peerDependenciesMeta" was not a thing yet :) Thanks to your PR this can be fixed now.

piotrwitek commented 1 year ago

I need to set up a new CI as the old one doesn't run the CI from forks, will merge soon, possibly tomorrow... If something distracts me please ping me again as a reminder. Thanks.

owenallenaz commented 1 year ago

For what it's worth, I've refactored my application so that it's using https://nodejs.org/api/packages.html#subpath-imports detailed here.

Previously I had code which depended on TS-declared "paths" like, and thus had to use tsconfig-paths (via --paths).

import Model from "@root/Model";

Now it's:

import Model from "#root/Model";

And I have "imports" declared in my package.json:

"imports": {
    "#root/*": "./src/*"
}

This has eliminated the need to utilize tsconfig-paths (or the --paths) CLI flag of ts-mocha, making this an issue thats no longer directly pressing to me, but I still think it's a worthwhile fix on your side, no reason to use an old-slower variant. The newer version is better for sure, but still worse than not even needing it 😁.

piotrwitek commented 1 year ago

Oh, that's really cool! Although this feature is still necessary for other consumers that are using paths config in their projects.

felipeplets commented 1 year ago

@piotrwitek is this PR good to merge? Let me know if you need support moving your old CI system to use GitHub Actions.

piotrwitek commented 1 year ago

Hey @felipeplets thanks a lot for the suggestion, I would be glad if you could spin up the GitHub Actions setup. Feel free to create PR, after it's done we'll be able to move forward with existing PRs.