search

runem / lit-analyzer

Monorepository for tools that analyze lit-html templates
MIT License
319 stars 38 forks source link

Implement function to detect facade modules; Adjust depth check in dependency traversal. #114

Closed FelixSchuSi closed 4 years ago

FelixSchuSi commented 4 years ago

This PR consists of two small changes:

  1. Fix of the dependency depth check. Currently Dependencies are always traversed one level too few. E. g. when MAX_EXTERNAL_DEPTH or MAX_INTERNAL_DEPTH are set to 1 not even directly imported modules (depth=1) are evaluated.
  2. Added funtion to detect facade modules. Currently external modules are evaluated two modules deep in order to support facade modules. Facade modules are modules that only consist of import and export declarations. These types of modules are frequently used in component libraries. With this change facade module are detected in dependency traversal and do not increase depth. Since facade modules dont increase depth anymore, MAX_EXTERNAL_DEPTH can be reduced to one. I have manually tested this with weightless components and carbon-custom-elements . It might be useful to take a look at other component libraries that use facade modules to see if the implemented function detects them corretcly.
FelixSchuSi commented 4 years ago

This is weird. The last commit contains the same changes as the first commit (except for a space), however the tests failed only for the first commit.

I couldnt find out why the tests failed, because all tests pass on my machine an the pipelines logs show an opaque node error.

runem commented 4 years ago

First of all, thank you so much for this PR! I look forwarding taking a closer look at the code later today when I get some more time. I expect to merge into 1.2.0 shortly thereafter :-)

It would be nice if you add some tests to test/parser/parse-dependencies.ts, but I can also add some tests afterwards :+1:

Unfortunately, currently package-locks are not used when installing package dependencies through Lerna, so the CI could be using minor/patch versions with potential bugs. It seems like a bug was introduced in ava yesterday that was fixed within a couple of hours with version 3.10.1.

runem commented 4 years ago

I just added two commits on top of your work :-)

(1) I added some more tests to parse-dependencies.ts.

(2) I would prefer the parameters "SourceFile" and "TsModule" to be camel cased and for "TsModule" to be named "ts" instead to follow project conventions, so I changed the names of the parameters.

https://github.com/FelixSchuSi/lit-analyzer/blob/ada458e82383038b7fdc1cc83b5b7ef53d4d994d/packages/lit-analyzer/src/analyze/parse/parse-dependencies/visit-dependencies.ts#L153-L165