search

open-wc / custom-elements-manifest

Custom Elements Manifest is a file format that describes custom elements in your project.
https://custom-elements-manifest.open-wc.org/
227 stars 37 forks source link

Support `--discoverNodeModules` option like WCA does it #123

Open k-paxian opened 2 years ago

k-paxian commented 2 years ago

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 commented 2 years ago

The 'lifecycle' of the analyzer is:

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 commented 2 years ago

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

image

https://github.com/open-wc/custom-elements-manifest/blob/cef1f46f745d7eb0edc4c42e31ab56a9e7543516/packages/analyzer/src/utils/cli.js#L28

thepassle commented 2 years ago

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 commented 9 months ago

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 ?