[New] add `--entrypoints-legacy` CLI command #156

ljharb · 2024-02-27T00:38:22Z

Note that the snapshots here are incorrect - despite these changes working as expected on the actual project/version in question, it's not working when i run the tests locally.

Closes #153.

changeset-bot · 2024-02-27T00:38:25Z

🦋 Changeset detected

Latest commit: ab6cd95

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@arethetypeswrong/cli	Minor
@arethetypeswrong/core	Minor
@arethetypeswrong/history	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/core/src/checkPackage.ts

andrewbranch

@arethetypeswrong/core always receives or fetches an npm tarball as an input. Isn’t using packlist/arborist redundant with that input? Can we just use all files it can see in the Package object?

packages/core/src/checkPackage.ts

ljharb · 2024-02-27T01:03:45Z

ah, hmm, i suppose that's true - i was assuming it runs in-situ, but i guess the pack option takes care of that?

I'll refactor to just look at the files, and that should remove the need for path as well as the added deps.

ljharb · 2024-02-27T01:34:11Z

thanks, that's much simpler. the snapshot still isn't matching the actual behavior of the built cli, tho.

andrewbranch · 2024-02-27T03:42:20Z

I checked this out locally, ran tests, and got a new snapshot:

❯ git diff --staged
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
added: packages/cli/test/snapshots/eslint-module-utils@2.8.1.tgz --entrypoints-legacy --ignore-rules=cjs-only-exports-default.md
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ packages/cli/test/snapshots/eslint-module-utils@2.8.1.tgz --entrypoints-legacy --ignore-rules=cjs-only-exports-default.md:4 @
# eslint-module-utils@2.8.1.tgz --entrypoints-legacy --ignore-rules=cjs-only-exports-default

```
$ attw eslint-module-utils@2.8.1.tgz --entrypoints-legacy --ignore-rules=cjs-only-exports-default


eslint-module-utils v2.8.1

Build tools:
- typescript@next

 (ignoring rules: 'cjs-only-exports-default')

 No problems found 🌟


┌─────────────────────────────────────────┬────────┬───────────────────┬───────────────────┬─────────┐
│                                         │ node10 │ node16 (from CJS) │ node16 (from ESM) │ bundler │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/declaredScope.js"  │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/hash.js"           │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/ignore.js"         │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/module-require.js" │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/ModuleCache.js"    │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/moduleVisitor.js"  │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/parse.js"          │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/pkgDir.js"         │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/pkgUp.js"          │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/readPkgUp.js"      │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/resolve.js"        │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/unambiguous.js"    │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
├─────────────────────────────────────────┼────────┼───────────────────┼───────────────────┼─────────┤
│ "eslint-module-utils/visit.js"          │ 🟢     │ 🟢 (CJS)          │ 🟢 (CJS)          │ 🟢      │
└─────────────────────────────────────────┴────────┴───────────────────┴───────────────────┴─────────┘


```

Exit code: 0
\ No newline at end of file

I think this looks right? What are you getting?

ljharb · 2024-02-27T03:43:22Z

That's totally right! what i get is what's in this PR :-/

ljharb · 2024-02-27T03:46:09Z

I was able to effectively start from scratch and get the md to update, but the json snapshot data still doesn't look right.

andrewbranch

Almost ready to go; if you wouldn’t mind adding something to the CLI README too, that would be great. Thanks!

packages/core/src/internal/getEntrypointInfo.ts

andrewbranch · 2024-02-27T16:49:16Z

packages/core/src/internal/getEntrypointInfo.ts

    if (proxies.length === 0) {
+      if (options?.entrypointsLegacy) {
+        return fs.listFiles()


I think it was an existing bug that there are early returns in here; I don’t see why either “proxy directories” or these files shouldn’t interact with--include-entrypoints and --exclude-entrypoints. I can rearrange all this in a different PR though if you don’t want to touch it here.

tbh I'm not actually sure what "proxy directories" is for, it seems to always be empty in my experience - as for include/exclude, i guess it does make sense to explicitly exclude things from the legacy list, but i'm not sure what the point would be for include - what would you include that's not already there?

Probably nothing, but maybe files that have non-standard/non-JS file extensions

andrewbranch · 2024-02-27T16:51:06Z

the json snapshot data still doesn't look right

The JSON snapshot is running the core library with default options; only the CLI test is getting the entrypointsLegacy option.

ljharb · 2024-02-27T17:37:48Z

Updated except for #156 (comment)

I'm happy to expand the functionality, in this PR, so that --exclude-entrypoints can be used in concert with --entrypoints-legacy - but it should probably be an error to use --entrypoints or --include-entrypoints with it, since there's nothing that can possibly be included that's not already in the file list?

andrewbranch · 2024-02-27T21:38:29Z

Thanks @ljharb!

[New] add --entrypoints-legacy CLI command

ab6cd95

ljharb commented Feb 27, 2024

View reviewed changes

packages/core/src/checkPackage.ts Outdated Show resolved Hide resolved

ljharb mentioned this pull request Feb 27, 2024

confusing resolution failure #153

Closed

ljharb force-pushed the legacy branch from 15b4d9a to e912d89 Compare February 27, 2024 00:41

andrewbranch reviewed Feb 27, 2024

View reviewed changes

packages/core/src/checkPackage.ts Outdated Show resolved Hide resolved

ljharb force-pushed the legacy branch from e912d89 to 9673c35 Compare February 27, 2024 01:33

ljharb force-pushed the legacy branch from 9673c35 to 6642693 Compare February 27, 2024 03:46

andrewbranch requested changes Feb 27, 2024

View reviewed changes

ljharb force-pushed the legacy branch from 6642693 to ab6cd95 Compare February 27, 2024 17:36

andrewbranch marked this pull request as ready for review February 27, 2024 18:02

andrewbranch merged commit 29d5811 into arethetypeswrong:main Feb 27, 2024
1 check passed

ljharb deleted the legacy branch February 27, 2024 22:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[New] add `--entrypoints-legacy` CLI command #156

[New] add `--entrypoints-legacy` CLI command #156

ljharb commented Feb 27, 2024

changeset-bot bot commented Feb 27, 2024 •

edited

Loading

andrewbranch left a comment

ljharb commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch commented Feb 27, 2024

ljharb commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch left a comment

andrewbranch Feb 27, 2024

ljharb Feb 27, 2024

andrewbranch Feb 27, 2024

andrewbranch commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch commented Feb 27, 2024

[New] add --entrypoints-legacy CLI command #156

[New] add --entrypoints-legacy CLI command #156

Conversation

ljharb commented Feb 27, 2024

changeset-bot bot commented Feb 27, 2024 • edited Loading

🦋 Changeset detected

andrewbranch left a comment

Choose a reason for hiding this comment

ljharb commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch commented Feb 27, 2024

ljharb commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch left a comment

Choose a reason for hiding this comment

andrewbranch Feb 27, 2024

Choose a reason for hiding this comment

ljharb Feb 27, 2024

Choose a reason for hiding this comment

andrewbranch Feb 27, 2024

Choose a reason for hiding this comment

andrewbranch commented Feb 27, 2024

ljharb commented Feb 27, 2024

andrewbranch commented Feb 27, 2024

[New] add `--entrypoints-legacy` CLI command #156

[New] add `--entrypoints-legacy` CLI command #156

changeset-bot bot commented Feb 27, 2024 •

edited

Loading