search

eslint / js

Monorepo for the JS language tools.
BSD 2-Clause "Simplified" License
2.29k stars 195 forks source link

feat: add the `eslint-scope` package #615

Closed snitin315 closed 2 months ago

snitin315 commented 3 months ago

Refers https://github.com/eslint/espree/issues/609

snitin315 commented 3 months ago

Updated 👍🏻

mdjermanovic commented 3 months ago

Looks like npm-license expects dependencies to be installed in node_modules of the start path. But in a workspaces monorepo, dependencies are installed in top-level node_modules.

When I add console.log(Object.keys(deps)); here, it logs only:

[ 'eslint-scope@8.0.2' ]

In the old eslint-scope repo, it logs:

[ 'eslint-scope@8.0.2', 'esrecurse@4.3.0', 'estraverse@5.3.0' ]

So it seems that the license check effectively wouldn't work? I'm not sure how this could be fixed and whether we need a license check for the eslint-scope package. We have a license check in eslint/eslint, and eslint-scope seems to be the only other package with this check.

nzakas commented 3 months ago

In a monorepo, all dependencies end up in the top-level node_modules, so if we do the license check from the root of the repo, I think it should work.

snitin315 commented 3 months ago

Providing the top-level path here also didn't work, it was then outputting [ 'eslint-js@1.0.0' ] only.

For now, I have passed additional metadata (See https://github.com/eslint/espree/pull/615/commits/64059749bc302488c024cb92099bc1709c824d9a) to keep the behavior the same as eslint-scope repo, I think we can follow up on this and see how it can be improved for monorepo.

nzakas commented 2 months ago

@snitin315 are you still working on this?

snitin315 commented 2 months ago

@nzakas Yes, somehow I missed the last review notification. I've updated the PR, PTAL.

@aladdin-add Let's change the eslint/eslint-scope to eslint/js in a follow-up PR for all packages, once the repository has been renamed.

nzakas commented 2 months ago

All right, moving this forward. I will rename the repo now.