-
Notifications
You must be signed in to change notification settings - Fork 26
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
base: master
Are you sure you want to change the base?
Conversation
Hey @owenallenaz, thanks for this PR. Indeed that was my intention but back in the day "peerDependenciesMeta" was not a thing yet :) |
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. |
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).
Now it's:
And I have "imports" declared in my package.json:
This has eliminated the need to utilize tsconfig-paths (or the --paths) CLI flag of |
Oh, that's really cool! Although this feature is still necessary for other consumers that are using paths config in their projects. |
@piotrwitek is this PR good to merge? Let me know if you need support moving your old CI system to use GitHub Actions. |
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. |
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 byts-mocha
and ourreact
andmaterial-ui
heavy stack. The older version oftsconfig-paths
has a very inefficient loading mechanism that results in hundreds of thousands of additional file system reads similar to this, taken usingstrace
: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 oftsconfig-paths
this changes to 3,000 file reads. This is due to another package increasing the possible extensions inrequire.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 apeerDependency
with it being marked asoptional
viapeerDependenciesMeta
(https://docs.npmjs.com/cli/v9/configuring-npm/package-json#peerdependenciesmeta). This means it will not be downloaded unless the user adds it to theirpackage.json
, and then it will use whatever version they download but will warn if they aren't using^4.1.1
.