search

Urthen / case-sensitive-paths-webpack-plugin

Enforces case sensitive paths in Webpack requires.
MIT License
428 stars 45 forks source link

WIP: make cache being actually used after a directory is read #53

Open frantic1048 opened 4 years ago

frantic1048 commented 4 years ago

This fixes issue #52.

Changes

Introduce async mutex to prevent calling fs.readdir() multiple times on the same path

This is the major content of this PR.

pathCache now becomes direct cache of fs.readdir() , regardless of being "good" or not. It just holds the result of fs.readdir().

There is a new Map visitedPathMutexRecord along with pathCache. It holds a mutex for each path we will visit, to achieve the following behavior.

Before the cache for a directory(let's say tippy here) is ready, we do the following process exclusively(non-parallelized):

  1. Check if the cache for tippy is ready and use the existing cache.
  2. Call fs.readdir() with tippy
  3. Write the result of fs.readdir() to cache. Cache for tippy is now ready.

After the cache of tippy is ready, there is only step 1, and it is being processed parallelized. on further calls to getFilenamesInDir() with directory tippy.

Apply lintfix in project

This is a separate commit at the first of all commits.

I found ESLint is warning during resolving the issue. And there are an existing ESLint config and a lintfix script in package.json

I think it is fair to apply it to prevent further distraction.

BTW, how about putting lint in CI jobs to keep things consistent?

frantic1048 commented 4 years ago

update: fixed test where callback is invoked multiple times on some case

Urthen commented 3 years ago

Hi there, Thanks for the contribution. I like the idea of speeding it up in general through parallelization but this isn't currently passing the tests. It looks like it's not properly failing if the folder case is wrong, at least that's the test failing on my machine.

Please make sure the tests pass locally and update.

frantic1048 commented 3 years ago

@Urthen Thanks for the inform. I revised the issue today and made a whole new fix. Of course, failing tests are fixed.

I updated the summary of changes in PR description, and added descriptions of key changes/questions in inline discussions.

Also, I resolved all outdated inline discussions opened by myself.

Review commits one by one is recommended since there is an ESlint styling fix commit.