TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined

[kaiyoma]

I have the latest version of this tool installed, but it crashes every time I run it:

$ skott --version
skott, 0.28.0

 Skott exited with code 0

$ skott -s

 Running Skott from current directory: event-viewer
⠇ Initializing Skott
 Skott exited with code 1
node:internal/errors:490
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:399:5)
    at validateString (node:internal/validators:163:11)
    at Object.join (node:path:429:7)
    at resolveAliasToRelativePath (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/modules/walkers/ecmascript/typescript/path-alias.js:10:36)
    at resolvePathAlias (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/modules/walkers/ecmascript/typescript/path-alias.js:88:35)
    at EcmaScriptDependencyResolver.resolve (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/modules/resolvers/ecmascript/resolver.js:113:40)
    at Skott.collectModuleDeclarations (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/skott.js:213:47)
    at async Skott.buildFromRootDirectory (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/skott.js:285:13)
    at async Skott.initialize (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/src/skott.js:305:13)
    at async skott (file:///C:/Users/kgetz/AppData/Roaming/nvm/v18.16.0/node_modules/skott/dist/index.js:27:27) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v18.16.0

[antoine-coulon]

Hello @kaiyoma, sorry to hear that it's still causing issues on your side.

During the time we have been discussing it on another issue which seems to be the same, unfortunately I was not able to reproduce the error during that time, hence the persistent issue. I added few error boundaries in #70 that should fix the runtime error even though it will not help resolving the underlying module (released in 0.28.1)

I remember that you provided the --verbose outcome in the past but as I added more fine-grained logs around path aliases in this PR it would be helpful to have logs with the new version (0.28.1).

Thanks

[kaiyoma]

Ah, my apologies, I forgot about the other issue. Feel free to close this as a duplicate if you want.

Some good news is that if I drop into the src/ sub-directory in my repo, then the skott analysis completes and finds a whole bunch of import cycles. Great success! I think this will actually work nicely for us and we probably don't need anything more from you.

In the interest of data sharing, here's the full verbose output from running skott in our repo root: skott.txt

I had to kill the command after a couple minutes to keep the logfile small enough to upload.

[antoine-coulon]

Indeed it's my mistake, I originally wrongly moved it to the Rush plugin repo, so it's fine there.

Some good news is that if I drop into the src/ sub-directory in my repo, then the skott analysis completes and finds a whole bunch of import cycles. Great success! I think this will actually work nicely for us and we probably don't need anything more from you.

Happy to hear that! I'd say it's indeed a good practice to target a bit more precisely files to be included in the graph either by using --cwd (moving directly into the folder) or using --ignorePattern, skott is black-list oriented meaning that it might explore too many unwanted things if run at the root of many files.

In the interest of data sharing, here's the full verbose output from running skott in our repo root: skott.txt

Thanks for the file sharing, it will for sure provide me good leads for further investigation.

[antoine-coulon]

Also @kaiyoma, as you mentioned that you were using the web application, feel free to share feature suggestions as I'm currently revamping it from scratch (see RFC #72), in the case where you or your collaborators felt like things were missing from the application (or skott itself).

Thanks in advance

[antoine-coulon]

Hello @kaiyoma, hope you're doing well.

Just wondering, are you still encountering the same set of problems? Also, don't hesitate to share me new information is you identified recurrent setups where skott was not working.

Thanks!

[kaiyoma]

No new updates right now, but I'll report back if I discover anything new.

[antoine-coulon]

Alright thanks for replying that fast as always. I'm letting the issue opened for now so that we can use this communication channel if needed.

[antoine-coulon]

Hello @kaiyoma, hope you're doing well :)

Lately I have been fixing few issues linked to Windows OS and as far as I can remember you had issues when running skott on this OS so if you're still using skott I would recommend you to jump on the latest version and try it out. You can then let me know how it goes and if you still encounter weird issues related to this topic

Thanks!

[kaiyoma]

@antoine-coulon I still see issues running skott from the root of our repo; instead of crashing, the new version seems to just hang forever. However, I don't think you should worry about this, because we have an awkward "hybrid repo" that is both a single-repo and a monorepo at the same time. (It's a temporary stage as we transition from the former to the latter.) If I drop down into the source folder for our "single repo", the tool works correctly and identifies a whole bunch of circular imports that I need to fix. 😄

[antoine-coulon]

@kaiyoma great to know thanks. How many files does that hybrid repo contain? Does it hang forever or is it just very very slow? When running on few thousands files (less than 10k) it runs in a matter of seconds but when having too much files it unfortunately does not scale well performance-wise, up to several minutes according some tests I did. I'll soon include some benchmarks on pretty heavy codebases to provide me a fair foundation on top of which I'll be able to optimize and improve many things.

But given that it does not crash, we might consider the current issue being related to efficiency/performance and not compatibility.

[kaiyoma]

@kaiyoma great to know thanks. How many files does that hybrid repo contain?

We have about 5,700 files in the "single repo" directory and about 11,400 files total.

Does it hang forever or is it just very very slow?

I'm not sure. I let it run for 5 minutes and it was constantly churning (with respect to CPU usage) and never finished. If I run skott on just the single source directory (the one with 5,700 files), it finishes in about a minute.

[antoine-coulon]

Thanks for the information :) This is more or less what I would expect given the current state of the tool.

It's also interesting to note that the number of files in itself is one parameter but then the number of relationships between files (that is graph edges) also come at a cost, so depending on the number of imports per module and the total number, the graph could be huge and slower to compute.

However it should never take more than few minutes for very heavy graphs imho, that's what I would expect if I was a skott user so improvements need to be done on that side.

Given you're very helpful as always I would like to take the opportunity to ask you one question: If I introduce a configurable flag to only and anonymously (code is fully open source) collect the time it took for skott’s graph (number of nodes and edges) to be computed to gather metrics and improve performance, would you enable it? If not, why? That data might be useful to track the performance path of skott.

Thanks!

[kaiyoma]

If I introduce a configurable flag to only and anonymously (code is fully open source) collect the time it took for skott’s graph (number of nodes and edges) to be computed to gather metrics and improve performance, would you enable it?

Sure. 😄

[antoine-coulon]

Roger that, thanks!

[mattkindy]

Regarding performance, I was looking at improving the performance on specifically front-end repositories which appeared to typically have larger files, more files, and more connections.

I was trying to suss out exactly what was causing performance to degrade in large graphs. From what I could see, it largely came down to re-doing already done work (opportunities for caching) and idle time due to serial vs concurrent processing. For example, without any changes:

After a couple specific changes here:

• Resolving modules concurrently
• Naively caching file contents in the file reader

I was able to almost cut that in half

I think these are fairly low-hanging fruit, and the memory impact of caching would need to be evaluated more rigorously (and also tested to avoid regressions on incremental/watch analysis), but would you be open to a PR along these lines @antoine-coulon ?

[antoine-coulon]

Hello @mattkindy,

Thanks for investigating! Performance is one of the next priorities I have in mind, this is why I started to add benchmarks in the CI to prepare the ground and being able to measure improvements over time.

Currently, lot of useless/avoidable work is done, I have few ideas in mind already, module resolution is one of them. As we say, performance can not only come from making things faster but also from getting rid of things (no thing will always be faster than a fast thing).

For the file system part, it's interesting, I'll take a look. Concurrency needs to be correctly managed for the graph to be consistent. When running skott from an entrypoint, I'm not sure we can make it more concurrent file-system wise as we incrementally discover the graph by visiting each node. However when using the bulk mode, that is traversing all files within a cwd, it might be worth it to enhance concurrency, I'll take a look at it.

Also @mattkindy you're always welcome to open PRs if you find any quick wins/improvements. I hadn't the time to do that before but it does not mean that I'm not willing to integrate free perf boosts ;)