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

perf: avoid unnecessary fs.statSync calls #206

Merged
merged 1 commit into from
Jan 11, 2023

Conversation

marvinhagemeister
Copy link
Contributor

Only resolve a module dir when we now that the mapped path is a directory. This improves linting time by about 18% in projects with lots of files.

The reason for the speedup is an accidental overhead in the callback of the filter function.

.filter(mappedPath => isFile(mappedPath) || isModule(mappedPath))

There we check if something resolves to file, and if not we'll check if it is a module directory. But in 99% of cases we'll be resolving file extensions in this callback. If /foo/bar/package.json is not a file, then it's very unlikely that /foo/bar.ts/package.json will be, unless /foo/bar.ts is a directory. Before this PR we'd check for the latter regardless which lead to an ENOTDIR error being thrown. The mere overhead of constantly throwing this error is where most of the time was spent in the traces I looked at.

Overall the logic can be improved further, but I think for now this PR reduces most of the overhead of calling into fs.statSync.

Before:

Screenshot 2023-01-10 at 15 09 11

After:

Screenshot 2023-01-10 at 15 09 16

@changeset-bot
Copy link

changeset-bot bot commented Jan 10, 2023

🦋 Changeset detected

Latest commit: a89bdbd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-import-resolver-typescript Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 10, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Only resolve a module dir when we now that the mapped path is a
directory. This improves linting time by about 18% in projects
with many files.
@JounQin JounQin merged commit 6531bad into import-js:master Jan 11, 2023
@marvinhagemeister marvinhagemeister deleted the mapped-perf branch January 11, 2023 09:50
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Aug 15, 2024
| datasource | package                           | from  | to    |
| ---------- | --------------------------------- | ----- | ----- |
| npm        | eslint-import-resolver-typescript | 3.5.1 | 3.6.1 |


##### [\`v3.6.1\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#361)

##### Patch Changes

-   [#241](import-js/eslint-import-resolver-typescript#241) [`cf5d67f`](import-js/eslint-import-resolver-typescript@cf5d67f) Thanks [@klippx](https://github.com/klippx)! - Fix CJS import to make it compatible with ESM projects
##### [\`v3.6.0\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#360)

##### Minor Changes

-   [#235](import-js/eslint-import-resolver-typescript#235) [`b5ea367`](import-js/eslint-import-resolver-typescript@b5ea367) Thanks [@SukkaW](https://github.com/SukkaW)! - refactor: drop `globby` and `synckit`
##### [\`v3.5.5\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#355)

##### Patch Changes

-   [`84b0649`](import-js/eslint-import-resolver-typescript@84b0649) Thanks [@JounQin](https://github.com/JounQin)! - fix: mark eslint-module-utils as dep
##### [\`v3.5.4\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#354)

##### Patch Changes

-   [`25f3920`](import-js/eslint-import-resolver-typescript@25f3920) Thanks [@JounQin](https://github.com/JounQin)! - fix: enhanced-resolve is commonjs only - close [#213](import-js/eslint-import-resolver-typescript#213)

-   [#219](import-js/eslint-import-resolver-typescript#219) [`0bf6ffb`](import-js/eslint-import-resolver-typescript@0bf6ffb) Thanks [@lsmurray](https://github.com/lsmurray)! - fix: check if cwd changed to bust mapper cache
##### [\`v3.5.3\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#353)

##### Patch Changes

-   [#206](import-js/eslint-import-resolver-typescript#206) [`6531bad`](import-js/eslint-import-resolver-typescript@6531bad) Thanks [@marvinhagemeister](https://github.com/marvinhagemeister)! - Only try to resolve a module directory when we know that the path is a directory. This can lead to a 15% speedup on projects with many files.
##### [\`v3.5.2\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#352)

##### Patch Changes

-   [#193](import-js/eslint-import-resolver-typescript#193) [`8756a26`](import-js/eslint-import-resolver-typescript@8756a26) Thanks [@Rialgar](https://github.com/Rialgar)! - chore(package): remove node 12 from engines field

-   [#187](import-js/eslint-import-resolver-typescript#187) [`7a91daf`](import-js/eslint-import-resolver-typescript@7a91daf) Thanks [@scott-ut](https://github.com/scott-ut)! - fix: resolve modules if folder contains a package.json file
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Aug 15, 2024
| datasource | package                           | from  | to    |
| ---------- | --------------------------------- | ----- | ----- |
| npm        | eslint-import-resolver-typescript | 3.5.1 | 3.6.1 |


##### [\`v3.6.1\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#361)

##### Patch Changes

-   [#241](import-js/eslint-import-resolver-typescript#241) [`cf5d67f`](import-js/eslint-import-resolver-typescript@cf5d67f) Thanks [@klippx](https://github.com/klippx)! - Fix CJS import to make it compatible with ESM projects
##### [\`v3.6.0\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#360)

##### Minor Changes

-   [#235](import-js/eslint-import-resolver-typescript#235) [`b5ea367`](import-js/eslint-import-resolver-typescript@b5ea367) Thanks [@SukkaW](https://github.com/SukkaW)! - refactor: drop `globby` and `synckit`
##### [\`v3.5.5\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#355)

##### Patch Changes

-   [`84b0649`](import-js/eslint-import-resolver-typescript@84b0649) Thanks [@JounQin](https://github.com/JounQin)! - fix: mark eslint-module-utils as dep
##### [\`v3.5.4\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#354)

##### Patch Changes

-   [`25f3920`](import-js/eslint-import-resolver-typescript@25f3920) Thanks [@JounQin](https://github.com/JounQin)! - fix: enhanced-resolve is commonjs only - close [#213](import-js/eslint-import-resolver-typescript#213)

-   [#219](import-js/eslint-import-resolver-typescript#219) [`0bf6ffb`](import-js/eslint-import-resolver-typescript@0bf6ffb) Thanks [@lsmurray](https://github.com/lsmurray)! - fix: check if cwd changed to bust mapper cache
##### [\`v3.5.3\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#353)

##### Patch Changes

-   [#206](import-js/eslint-import-resolver-typescript#206) [`6531bad`](import-js/eslint-import-resolver-typescript@6531bad) Thanks [@marvinhagemeister](https://github.com/marvinhagemeister)! - Only try to resolve a module directory when we know that the path is a directory. This can lead to a 15% speedup on projects with many files.
##### [\`v3.5.2\`](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#352)

##### Patch Changes

-   [#193](import-js/eslint-import-resolver-typescript#193) [`8756a26`](import-js/eslint-import-resolver-typescript@8756a26) Thanks [@Rialgar](https://github.com/Rialgar)! - chore(package): remove node 12 from engines field

-   [#187](import-js/eslint-import-resolver-typescript#187) [`7a91daf`](import-js/eslint-import-resolver-typescript@7a91daf) Thanks [@scott-ut](https://github.com/scott-ut)! - fix: resolve modules if folder contains a package.json file
renovate bot added a commit to andrei-picus-tink/auto-renovate that referenced this pull request Aug 15, 2024
| datasource | package                           | from  | to    |
| ---------- | --------------------------------- | ----- | ----- |
| npm        | eslint-import-resolver-typescript | 3.5.1 | 3.6.1 |


## [v3.6.1](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#361)

##### Patch Changes

-   [#241](import-js/eslint-import-resolver-typescript#241) [`cf5d67f`](import-js/eslint-import-resolver-typescript@cf5d67f) Thanks [@klippx](https://github.com/klippx)! - Fix CJS import to make it compatible with ESM projects


## [v3.6.0](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#360)

##### Minor Changes

-   [#235](import-js/eslint-import-resolver-typescript#235) [`b5ea367`](import-js/eslint-import-resolver-typescript@b5ea367) Thanks [@SukkaW](https://github.com/SukkaW)! - refactor: drop `globby` and `synckit`


## [v3.5.5](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#355)

##### Patch Changes

-   [`84b0649`](import-js/eslint-import-resolver-typescript@84b0649) Thanks [@JounQin](https://github.com/JounQin)! - fix: mark eslint-module-utils as dep


## [v3.5.4](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#354)

##### Patch Changes

-   [`25f3920`](import-js/eslint-import-resolver-typescript@25f3920) Thanks [@JounQin](https://github.com/JounQin)! - fix: enhanced-resolve is commonjs only - close [#213](import-js/eslint-import-resolver-typescript#213)

-   [#219](import-js/eslint-import-resolver-typescript#219) [`0bf6ffb`](import-js/eslint-import-resolver-typescript@0bf6ffb) Thanks [@lsmurray](https://github.com/lsmurray)! - fix: check if cwd changed to bust mapper cache


## [v3.5.3](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#353)

##### Patch Changes

-   [#206](import-js/eslint-import-resolver-typescript#206) [`6531bad`](import-js/eslint-import-resolver-typescript@6531bad) Thanks [@marvinhagemeister](https://github.com/marvinhagemeister)! - Only try to resolve a module directory when we know that the path is a directory. This can lead to a 15% speedup on projects with many files.


## [v3.5.2](https://github.com/import-js/eslint-import-resolver-typescript/blob/HEAD/CHANGELOG.md#352)

##### Patch Changes

-   [#193](import-js/eslint-import-resolver-typescript#193) [`8756a26`](import-js/eslint-import-resolver-typescript@8756a26) Thanks [@Rialgar](https://github.com/Rialgar)! - chore(package): remove node 12 from engines field

-   [#187](import-js/eslint-import-resolver-typescript#187) [`7a91daf`](import-js/eslint-import-resolver-typescript@7a91daf) Thanks [@scott-ut](https://github.com/scott-ut)! - fix: resolve modules if folder contains a package.json file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants