Skip to content

Make maxInternalDepth and maxExternalDepth configurable.#116

Merged
runem merged 5 commits intorunem:1.2.0from
FelixSchuSi:1.2.0-traversal-depth-config
Jul 13, 2020
Merged

Make maxInternalDepth and maxExternalDepth configurable.#116
runem merged 5 commits intorunem:1.2.0from
FelixSchuSi:1.2.0-traversal-depth-config

Conversation

@FelixSchuSi
Copy link
Contributor

I made maxInternalDepth and maxExternalDepth configurable.
The values can now be set under the name "moduleTraversalDepthInternal" and "moduleTraversalDepthExternal" via CLI argument, tsconfig or VSCode setting.

The default values stayed the same ("moduleTraversalDepthInternal" defaults to Infinity and "moduleTraversalDepthExternal" defaults to one).

Handling the value Infinity in JSON is tricky since it is not supported by the JSON format.
I decided to allow string values and cast them to number when loading the config, so that the value Infinity can be represented with the string "Infinity".

I am not completely satisfied with the variable names and the descriptions.
Please tell me what you think about them.
Once we have agreed on the variable names I will add documentation.

@runem
Copy link
Owner

runem commented Jul 11, 2020

Infinity

I think using the string "Infinity" could become a bit inconvenient. What do you think about using -1 instead of "Infinity"?

Naming

I actually really like the description you made for the configs :-)

Here are some thoughts about the naming of the configs:

  • Using the word "import" instead of "traversal" could make the meaning more clear because "depth" already indicates some kind of traversal.
  • The word "depth" should be included
  • The word "max" could be useful to indicate that this is the maximum allowed depth to traverse.
  • Typescript has a configuration called maxNodeModuleJsDepth: The maximum dependency depth to search under node_modules and load JavaScript files.
  • ESLint has a configuration called max-depth: Enforce a maximum depth that blocks can be nested

Here are some suggestions (unordered list) of what it could be called. What do you think?

  • maxExternalImportDepth & maxInternalImportDepth
  • maxExternalDependencyDepth & maxInternalDependencyDepth
  • maxNodeModuleImportDepth & maxProjectImportDepth
  • moduleImportDepthInternal & moduleImportDepthExternal

Documentation

When you add documentation you will need to:

  1. Edit ./docs/readme/config-table.md
  2. npm run readme
  3. npm run prettier:write

@FelixSchuSi
Copy link
Contributor Author

Thanks for the feedback!

Using -1 instead of "Infinity" is definitely a nicer way, I will change that soon!

For the names of the configs i found maxNodeModuleImportDepth & maxProjectImportDepth to be my favourite.
I think that "NodeModule" and "Project" are a bit more intuitive than "internal" and "external".

@runem
Copy link
Owner

runem commented Jul 13, 2020

I also like them. Let's go with those two then!

@runem
Copy link
Owner

runem commented Jul 13, 2020

It looks great now, - I'll merge this in a couple of hours!

@runem runem merged commit 8e29ceb into runem:1.2.0 Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants