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

Support --discoverNodeModules option like WCA does it #123

Open
k-paxian opened this issue Sep 28, 2021 · 4 comments
Open

Support --discoverNodeModules option like WCA does it #123

k-paxian opened this issue Sep 28, 2021 · 4 comments
Labels
good first issue Good for newcomers

Comments

@k-paxian
Copy link

web-component-analyzer is allowing to drill down to node_modules folder, it's essential functionality for my project and it's a show stopper for this tool.

https://github.com/runem/web-component-analyzer/blob/1a71064c66a38923aab783e3885feefccfd19589/src/cli/analyzer-cli-config.ts#L23

@thepassle
Copy link
Member

The 'lifecycle' of the analyzer is:

  • Get all globs to analyze
  • Compile them with TS
  • Analyze them

We can only know which imports/node_modules to analyze when we're analyzing the source code, at which compilation has already happened. Currently there's no good solution to this yet.

As a temporary workaround, you can specify the dependencies/node_modules you want to analyze in the globs property of the config, e.g.:

export default {
  globs: ['node_modules/my-dependency/**/*.js'], 
}

@k-paxian
Copy link
Author

Thanks for fast response, still not sure this will help to override last entry the way you suggest

image

@thepassle
Copy link
Member

thepassle commented Sep 28, 2021

ah yes we should allow that then. But we'd also need to ignore node_modules by default

Edit: Maybe we should have some similar logic, if the user has provide a glob including node_modules/, they want to analyze dependencies(/override the default globs), and then they're responsible for including/excluding the dependencies they need to themself. (Which then should be clearly documented)

@wtnabe
Copy link

wtnabe commented Oct 21, 2023

Hi, there.

I would like to use https://github.com/break-stuff/cem-tools/tree/main/packages/vs-code-integration to writing Custom Elements with https://github.com/material-components/material-web ( manifest file not supplied ) , but due to the limitations of this issue, I have not been able to do so.

Since custom-elements-manifest analyzer@0.9.0 uses globby, I tested it with `globby' alone.

https://gist.github.com/wtnabe/bedac43ed85b5ad6f71e6d3b78edb6ad#file-result-txt

As a result, I found that the current IGNORE position does not allow to retrieve files under node_modules/.

It would be better to put the IGNORE at the beginning of the array if it is the default, but it is true that giving a pattern like **/*.js to globs ignores the IGNORE pattern, which is not desirable because it will take a lot of time to analyze.

I have come up with a solution based on the assumption that globs and exclude are given to `globby' as they are. It will make the following three changes:

  1. Put IGNORE at the beginning of merged.
  2. Prepare appropriate defaults for globs (ex, globs: [ './src/**/*.js' ] )
  3. Provide a link to globby in the documentation.

In most cases, this should work fine. If there is no need to change the settings in globs, then we will not unintentionally run a COLLECT PHASE on all files under node_modules/.

How about this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants