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

Allow filtering of full modules tree #282

Closed
wants to merge 1 commit into from
Closed

Conversation

Swaagie
Copy link

@Swaagie Swaagie commented Apr 1, 2021

The goal of this PR is enable dependencyFilter to filter all modules rather than just a single level from the tree. While trying to be backwards compatible with current implementations as much as possible. Backwards compatibility can't be achieved for those who use a filter with includeNpm.

Use cases are for example filtering all scoped dependencies, especially if those scoped dependencies depend on other modules within the same scope. Imagine a monorepo where composites are derived from a set of base components within the same monorepo. Technically Madge could be run for each dependency but getting the full import tree would be cumbersome that way.

There are two alternatives to this solution and I'm happy to change the code towards the desired direction.

  • config.dependencyFilter overrides the default filter implementation. This provides flexibility but will not be backwards compatible. Anyone using a filter today would need to include the defaults. The default filter could easily be exposed for re-use within custom filters. Personally this would have my preference as its much easier to document, code and keep consistent.
  • additional option to include the entire modules tree rather than a single level, or perhaps repurpose includeNpm to allow for a certain depth of n with a default of 1.

@Swaagie Swaagie changed the title Allow filtering of all node_modules Allow filtering of full modules tree Apr 1, 2021
@PabloLION
Copy link
Collaborator

Hi @Swaagie , thanks for the hard work. But it seems we are having a problems here

rc problem

As reported by

  1. unmeet the rc standard
  2. has no effect

We might need to rework on this. I don't have a mature idea about the implementation of how to pass a JS function via CLI/rc file.

Based on my very limited edification, this is about how to serialize the function and it seems this is the best way to do it?

New ideas

An alternative worth mentioning is to use RegExp like we did with excludeRegExp, we can filter the npm modules with npmFiles, npmInclude, npmExclude (or npmFilesRegExp, npmIncludeRegExp, npmExcludeRegExp) like #353 (formerly #220) to line up with the tsconfig convention.

Any opinion would be appreciated. @kamiazya @fdc-viktor-luft @pahen
@eltonio450(does this achieve the idea of npmWhitelist in #281?)

@PabloLION
Copy link
Collaborator

closed due to inactivity. Welcome to create another PR if needed

@PabloLION PabloLION closed this Sep 30, 2023
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