feat(eslint-plugin): [no-var-requires, no-require-imports] allow option

[Josh-Cena]

PR Checklist

• Addresses an existing open issue: fixes [no-var-requires] add an option to ignore require('../package.json') #1902, fixes Enhancement: [no-require-imports] Add option allowJson #5314
• That issue was marked as accepting prs
• Steps in Contributing were taken

Overview

[typescript-eslint]

Thanks for the PR, @Josh-Cena!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

[netlify]

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	b77f61d
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/6580a1a0ca656500085b0b41
😎 Deploy Preview	https://deploy-preview-7710--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (no change from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cc3c317. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

🟥 Failed Commands
nx test eslint-plugin --coverage=false

✅ Successfully ran 19 targets

• nx run-many --target=lint --parallel
• nx test type-utils --coverage=false
• nx test typescript-estree --coverage=false
• nx test visitor-keys --coverage=false
• nx run types:build
• nx test utils --coverage=false
• nx test eslint-plugin-tslint --coverage=false
• nx test repo-tools --coverage=false
• nx test scope-manager --coverage=false
• nx run repo-tools:postinstall-script
• nx test rule-schema-to-typescript-types --coverage=false
• nx test parser --coverage=false
• nx test eslint-plugin-internal --coverage=false
• nx test ast-spec --coverage=false
• nx run-many --target=typecheck --parallel
• nx run integration-tests:test
• nx build website
• nx run eslint-plugin:generate:configs
• nx run-many --target=build --parallel --exclude=website --exclude=website-eslint

---

Sent with 💌 from NxCloud.

[Josh-Cena]

#5314 asks for allowJson. This PR narrows the scope, because in fact most JSON can be correctly imported in Node, as long as it lives inside src (although you would have to use import attributes, which is highly experimental). Not sure if we should expand the scope to cover all JSON.

[JoshuaKGoldberg]

I've also seen libraries want to import from turbo.json, project.json, etc. Maybe this should be the standard format from #6017, with docs showing how to use it for package.json files? WDYT?

[Josh-Cena]

I'm okay with that. What's your proposed API? My thought is something similar to no-restricted-imports, with an array of paths.

[JoshuaKGoldberg]

I was thinking the exact format from #4436 -> https://typescript-eslint.io/rules/prefer-readonly-parameter-types.

{
  "@typescript-eslint/no-var-requires": [
    "error",
    {
      ignore: [
        {
          name: "package.json",
          source: "file",
        },
      ],
    },
  ],
}

[Josh-Cena]

This looks couunterintuitive as a user, TBH. This API is for types, but what is "package.json" as a type? What does source: "file" even mean? With no-restricted-imports, the API would look like:

{
  "@typescript-eslint/no-var-requires": [
    "error",
    {
      "ignorePaths": [
        {
          "name": "*.json"
        }
      ]
    }
  ]
}

[JoshuaKGoldberg]

What happens if they want to import from a node module, e.g. import emojis from "emojilib/dist/emoji-en-US.json"? Should we just have a string[] as the option to capture all those cases? Feels off to me that we have the big thought-out format and then don't use it... but it is overhead, yes. 🤔

[JoshuaKGoldberg]

ping @Josh-Cena ?

[Josh-Cena]

Sorry, I did not forget this 😅 Will try to garner some energy and clear my notifications this afternoon.

[Josh-Cena]

@JoshuaKGoldberg I thought about it. Here are my thoughts:

1. The only thing this PR changes is allowing require. It doesn't change anything about import—they can still be used as normal.
2. Sure, we have a thought-out format for specifying types, but here we are talking about import paths, and this design doesn't come without its precedent either—we are being consistent with no-restricted-imports.
3. About having string[] as an option. You mean { ignorePaths: string[] }? My initial design is { ignorePaths: TheOptionsOfNoRestrictedImports }, i.e. it supports multiple ignore paths, each one with a path, a glob, a custom message, etc. But I now think that's hugely unnecessary for an ignore option. So sure, I can do that.
4. If you want to differentiate between node modules and local JSON files, you would have to support regex in the ignore path (I think) to be able to tell between ./foo/data.json and foo/data.json. That means the option has to be more granular than a string[]; it would at minimum need to be { ignorePaths: { paths: string[] /* globs */, patterns: string[] /* regexes */ }. Does that make sense? Am I overthinking it?

I'm very curious what your current idea for the option's shape is.

[JoshuaKGoldberg]

The intent of #6017 was: "RFC: Common format to specify a type or value as a rule option". I was really hoping it'd be usable for situations exactly like this one. But if we're seeing a practical shortcoming of the format -it being too heavy-handed- then I like this as a motivation to improve it. Or maybe it's just irrelevant here...

Maybe just allow: string[] or ignore: string[], where each of the string elements is a regex for now - and only if users ask we expand to that format? It'd be a little verbose to have to say something like "package\.json$"], but not the worst thing in the world IMO.

Thinking on the sticking point of relativity: from what I can think of either:

• the rule gets told a root ESLint path to resolve from (but what about monorepos with config in the root vs per-package?)
• we tell users to either increase regex specificity or allow sibling level package.json imports (/[.\\\/]*package.json/)

I think having two things to think about, like paths and patterns from 4., would be a bit much. Is there anything that can only be captured in that system, not the single option?

[bradzacher]

The complicated thing is that sometimes you have tsconfig paths which make things look non-relative. We can't build in any specific assumptions around the path here if we want people to allow local but disallow node_module packages done this way.

So a regex is probably the most flexible option for this so codebases can easily devise some regex that matches their usecase. I'd suspect that most codebases would just want something like you suggested joshg to allow a relative package.json

[Josh-Cena]

After #8092 we can finally get rid of this sucky duplication...

[StyleShit]

does it work with string constants?

const path = 'package.json';
const pkg = require(path);

[Josh-Cena]

No, that needs to make the rule type-aware, or otherwise implement a go-to-definition mechanism, which I'm not sure we have the precedent. I'm also not sure if it happens more than a handful of times IRL.

[JoshuaKGoldberg]

LGTM! The template literal support I trust you on 😄 it's not high priority IMO. Otherwise this looks great, thanks!

[JoshuaKGoldberg]

[Bug] I think this should use getStaticStringValue or an equivalent tailored just to strings. Then it would support template literals.

I did a quick search for type === AST_NODE_TYPES.Literal and found a few files Referencer.ts, prefer-regexp-exec.ts, prefer-reduce-type-parameter.ts, padding-line-between-statements.ts, no-restricted-imports.ts...) that do similar "get the value if it's a string literal" logic. Maybe good to make a new utility?

Thoughts?

[JoshuaKGoldberg]

#8186. I wanted to get this PR merged 😄 and I don't think template literals in require() calls are pretty rare.