Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump tsconfig-paths and place in peerDependencies #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

owenallenaz
Copy link

@owenallenaz owenallenaz commented Jan 3, 2023

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
Copy link
Owner

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
Copy link
Owner

piotrwitek commented Jan 3, 2023

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
Copy link
Author

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
Copy link
Owner

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

@felipeplets
Copy link
Contributor

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

@piotrwitek
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants