[APPS] Add @datadog/eslint-plugin-apps with valid-connections-file rule#332
[APPS] Add @datadog/eslint-plugin-apps with valid-connections-file rule#332sdkennedy2 wants to merge 2 commits into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f66f3a66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "peerDependencies": { | ||
| "eslint": ">=8.57.0" | ||
| }, |
There was a problem hiding this comment.
Declare the TypeScript parser for consumers
For the documented connections.ts setup, both README configs require @typescript-eslint/parser, but this package only declares eslint as a peer and keeps the parser in devDependencies. In a fresh app that follows npm install --save-dev @datadog/eslint-plugin-apps, the consumer won't get this package's dev dependency, so the flat-config import or legacy parser resolution fails before the rule can run. Add the parser to the consumer-facing install path (for example a peer/optional peer plus docs) or remove the requirement.
Useful? React with 👍 / 👎.
| const basename = path.basename(context.filename); | ||
| if (!CONNECTIONS_BASENAME_RE.test(basename)) { |
There was a problem hiding this comment.
Scope the rule to the root connections file
Because the recommended config enables this rule globally, checking only path.basename(context.filename) makes any nested src/connections.ts or packages/foo/connections.js subject to the Apps root-file contract. The Vite plugin finder only reads connections.{ts,tsx,js,jsx} directly under buildRoot, so nested files are ignored at build time but now fail lint with missingExport or shape errors. Scope the recommended config/rule to the configured root file to avoid these false positives.
Useful? React with 👍 / 👎.
| import validConnectionsFile from './rules/valid-connections-file'; | ||
|
|
||
| const PLUGIN_NAME = 'apps'; | ||
| const PLUGIN_PACKAGE = '@datadog/eslint-plugin-apps'; |
There was a problem hiding this comment.
Derive plugin metadata version from package.json
This hard-coded version will not be updated by the repo's yarn version:all workflow, which only bumps package manifests, so the published plugin metadata becomes stale after the next release even though ESLint expects plugin meta.version to match the package. That can make --print-config misleading and can keep --cache from noticing plugin updates; derive it from package.json like the other published packages do.
Useful? React with 👍 / 👎.
735769b to
9362c60
Compare
6f12a4e to
b64302d
Compare
9362c60 to
0e68960
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e68960f27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ## Install | ||
|
|
||
| ```bash | ||
| npm install --save-dev @datadog/eslint-plugin-apps |
There was a problem hiding this comment.
Install the parser used by the documented configs
In a fresh consumer project following this install command, both documented configs immediately reference @typescript-eslint/parser (the flat config imports it and the legacy config names it), but this package only declares that parser as a devDependency, so it is not installed or peer-checked for consumers. This makes the README setup fail with a missing parser as soon as users lint a TypeScript connections.ts; include the parser in the install instructions or declare an appropriate peer/optional peer.
Useful? React with 👍 / 👎.

Motivation
Stacked on top of #331. That PR introduced the build-time
extract-connections.tsextractor that fails the build when a project'sconnections.tsis malformed. PR #331 explicitly defers an ESLint rule for the same shape to a follow-up — this is that follow-up.The rule mirrors the extractor's failure cases one-for-one so customers see violations in their editor instead of as a build error. Living in the same repo as the extractor means the two stay in lockstep — extractor contract changes land in the same PR as the rule that mirrors them.
Changes
New publishable npm package
@datadog/eslint-plugin-appsatpackages/published/eslint-plugin-apps/. Builds with rollup (custom config —getDefaultBuildConfigsis bundler-plugin specific and would mis-parse the package name), emits dual ESM (.mjs) + CJS (.js) +.d.tstodist/src/. Versioned in lockstep with the rest of the@datadog/*packages (3.1.4) soyarn version:allkeeps everything aligned.Customers wire it up with either flat config or legacy
.eslintrc:The package is structured to grow: more rules can be added under
src/rules/<name>.tsand registered insrc/index.ts.Initial rule:
valid-connections-fileMirrors the extractor at
packages/plugins/apps/src/backend/extract-connections.ts. Auto-scopes by basename — only runs on files matchingconnections.{ts,tsx,js,jsx}. Reports:export const CONNECTIONS = {…}missingExportas constetc.)notObjectLiteralexport const CONNECTIONSduplicateExportspreadElementcomputedKeyvalueNotStaticStringtemplateInterpolationOnly
CONNECTIONS(uppercase) is accepted, matching the extractor. The rule unwrapsTSAsExpression/TSTypeAssertion/TSSatisfiesExpression/TSNonNullExpressionbefore structural checks since@typescript-eslint/parserpreserves those as AST nodes.Notable build details
format: 'cjs'output,external: ['eslint', 'node:path', 'path']— eslint plugins must berequire()-loadable for legacy.eslintrc. Rollup's CJS emit for anexport defaultsource already producesmodule.exports = plugindirectly, so nomodule.exports.defaultfooter trick is needed (that workaround is only required when the bundler is esbuild).rollup-plugin-dts).buildPlugin.hideFromRootReadme: truekeepsyarn cli integrity's root-README plugin table from gaining a row that doesn't fit (it's wired for bundler plugins).@types/estreetypes internally for traversal (real type-checking on field access) and castsas unknown as Rule.Nodeonly at the fewcontext.reportcall sites. The reason for the casts:@types/eslint@9.6.1ships a nested copy of@types/estree@1.0.7, while the rest of the repo (rollup, webpack,@dd/apps-plugin) pins1.0.8. The two physical type modules are structurally identical but TypeScript treats them as distinct identities, so aVariableDeclaratorfrom our top-level estree doesn't satisfyRule.Node(which references@types/eslint's nested copy). Pinning our package to1.0.7doesn't help either — Node's module resolution still finds a different physical install path adjacent to our package vs. the one nested under@types/eslint. A small, localized cast at the report boundary is the cleanest fix; anasRuleNodehelper documents the intent in one place.QA Instructions
yarn workspace @datadog/eslint-plugin-apps build— emitsdist/src/index.{js,mjs,d.ts}.yarn workspace @dd/tests jest packages/published/eslint-plugin-apps/src/rules/valid-connections-file.test.ts— 13 RuleTester cases pass (4 valid + 9 invalid, including a "lowercaseconnectionsis not accepted" assertion).yarn lint,yarn typecheck:all— clean.yarn cli integrity— clean (no required updates).yarn pack --out /tmp/eslint-plugin-apps.tgzfrompackages/published/eslint-plugin-apps/.npm install --no-save /tmp/eslint-plugin-apps.tgz @typescript-eslint/parser eslint@9in the test app.connections.ts(missing export, lowercaseconnections, non-object init, duplicate, spread, computed key, identifier value, template interpolation). Each fires its correspondingmessageIdat the offending line:col.node -e "console.log(Object.keys(require('@datadog/eslint-plugin-apps').rules))"→[ 'valid-connections-file' ].Blast Radius
@datadog/*package picks up the existingversion:alland release workflow automatically.yarn cli integritydoesn't auto-add a row to the root README's plugin table thanks tobuildPlugin.hideFromRootReadme: true. Future contributors browsing the table won't be misled into thinking this is a bundler plugin.