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

Add support for the --include flag #353

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danburzo
Copy link

Fixes #219. This is a redo of a previous PR:

@PabloLION
Copy link
Collaborator

This might be too much to ask: could you please include some tests to show this PR works as intended? @danburzo

@danburzo
Copy link
Author

No worries, I'll be glad to include some tests when I find a window of time!

In regards to your proposal to mirror .tsconfig conventions, I think because --exclude already works differently from .tsconfig.exclude, making an analogy is not possible.

Important: exclude only changes which files are included as a result of the include setting. A file specified by exclude can still become part of your codebase due to an import statement in your code, a types inclusion, a /// <reference directive, or being specified in the files list.

It is not a mechanism that prevents a file from being included in the codebase - it simply changes what the include setting finds.

I'll rethink the names of the CLI options to see if there's something better available.

@PabloLION
Copy link
Collaborator

I agree. The current dependencyFilter is kinda messy (see #282) so it's probable that we change everything to mirror tsconfig.
I'm not sure how current --exclude differs from tsconfig. For me they both exclude some "modules" to be parsed/checked by tsc/madge (because tsconfig has include default to "[] if files is specified, ** otherwise." )

One difference I noticed is the glob-pattern and RegExp. I can't decide which one is better:

  • RegExp is much more expressive
  • glob-pattern has less version / flavor, hence more stable

Maybe we can first keep everything the same to finish this PR? We welcome all to discuss about the new --include and --exclude in a new issue.

@danburzo
Copy link
Author

danburzo commented Jan 30, 2023

I'm not sure how current --exclude differs from tsconfig

If I understand correctly (not a TS user), .tsconfig's include, exclude, and files options control the set of input files. All imports are then added to the list of files to process.

In contrast, include and exclude as defined in this PR (as well as the current definition of exclude) act together as a filter to what's rendered in the graph. The set of input files are defined as the operands to the CLI interface: madge file1 file2 ... and expanded based on their dependencies, then filtered by include/exclude: they have to match an include pattern and not be excluded by an exclude pattern.


function regExpFilter(id) {
return regExpList.findIndex((regexp) => regexp.test(id)) < 0;
return (!includeList || includeList.findIndex((re) => re.test(id)) >= 0) &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic means, only filenames declared in --include are included, other files, even those not --exclude-d, won't be checked?

@PabloLION
Copy link
Collaborator

@danburzo, sorry 😅 I was too stupid. I just realized: in tsconfig, include and exclude only changes the entry points rather than files transpiled by typescript.

What I wanted to say at first was to give priorities to include and exclude: like tsconfig.json first add all files whose filename fits the include pattern, then remove those of exclude.
Maybe we should implement something like this?

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.

Idea: --only / --include flag?
2 participants