Conversation
As required by ESLint 9: see https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#changes-to-how-you-write-rules
neuracr
added a commit
to google/safety-web
that referenced
this pull request
Aug 6, 2024
Had to downgrade ESLint to v8 since the plugin is not compatible with ESLint9 (yet?) Stuk/eslint-plugin-header#56 Change-Id: I85a07ce19f1a8c30ff65f2c010edea81bc9366f0
abzats
approved these changes
Sep 19, 2024
|
@joshkel I have published the previous incremental fix to unblock work with ESLint9 - see #58. As I didn't get an approval on the PR, I had to publish from my fork: https://www.npmjs.com/package/@tony.ganchev/eslint-plugin-header. Would you mind creating a PR for this fork until the owners of the original repo fix the CI/CD (if I understand the status of this PR correctly...) |
tonyganchev
pushed a commit
to tonyganchev/eslint-plugin-header
that referenced
this pull request
Jun 20, 2025
Original PR: Stuk/eslint-plugin-header#56 As required by ESLint 9: see https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#changes-to-how-you-write-rules
tonyganchev
pushed a commit
to tonyganchev/eslint-plugin-header
that referenced
this pull request
Jun 20, 2025
Original PR: Stuk/eslint-plugin-header#56 As required by ESLint 9: see https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#changes-to-how-you-write-rules
Author
Hey, @tonyganchev. Thanks for creating that fork. I apologize for not following up sooner. If I understand correctly, it looks like you already took care of this? |
That's right. Thanks, and pardon me for not waiting for the response :) |
jessieweiyi
pushed a commit
to awslabs/threat-composer
that referenced
this pull request
Sep 21, 2026
* build: Eject projen with npx projen eject Run the eject task on main, unchanged, so the hand-off from projen is recorded as a single reviewable commit. projen removed its "Generated by projen" markers, made the generated files writable, dropped the default and eject tasks, removed projen (and the constructs dependency it owned) from package.json, deleted .projen/deps.json and .projen/files.json, and added a scripts/run-task shim at the root and in every package so the existing tasks.json graph keeps running. The per-package project.json files keep their nx targets, now pointing at the shim instead of yarn projen. Two things eject did not finish. Its postinstall step failed because the shim it wrote for threat-composer-ai, a uv project with no package.json, requires ../package.json (projen/projen#1563), so the final rename of .projenrc.ts to .projenrc.ts.bak is done here by hand. It also wrote requirements.txt and requirements-dev.txt for the Python project, which the repository's own UvPythonProject used to strip after synthesis. Both are removed in the next commit, which replaces the shim with direct commands. Nothing under packages/*/src changes. * build: Replace the projen task shim with nx targets Each package's project.json now declares its steps as separate nx targets with direct commands, instead of every target calling scripts/run-task to replay .projen/tasks.json. The shims, tasks.json, .projenrc.ts.bak, projenrc/ and the requirements*.txt eject wrote for the uv project are deleted, as are the @aws/pdk root devDependency and the projen-specific entries in the ignore and attribute files. Task granularity follows the old projen tasks, split where a step did two things: @aws/threat-composer has clean, compile, copy-assets, storybook:build, eslint, test and package; the app has clean, one compile:* target per artefact, copy-storybook, eslint and test; the browser extension has clean, compile:chrome, compile:firefox, eslint, test and package; infra has clean, synth, eslint and test; threat-composer-ai has install, lint, lint:fix and test. Every build target is an nx:noop whose dependsOn lists those targets plus ^build, so `yarn build` (nx run-many -t build) runs eslint, typecheck, ruff and every test suite as part of the graph rather than as separate CI steps. Cross-package edges that used to be implicit in CI step order are now target dependencies: the app's eslint and test depend on the library's compile and copy-assets; copy-storybook depends on the library's storybook:build; the extension's compile:chrome depends on the app's compile:browser-extension; infra synth and test depend on ^build because the stacks read the app's build directory at synth time. compile:firefox depends on compile:chrome so two wxt builds never run concurrently in one package. The repository root is itself an nx project (project.json) owning build:packs and build:schema, which write into the library's src and schemas/, plus export:examples, generate:attribution and license:checker. The library's compile depends on build:packs, which was previously guaranteed only by the order of the root build script. e2e joins the yarn workspaces and becomes an nx project. Its e2e target depends on @aws/threat-composer:copy-assets, replacing the compile and rsync steps CI carried by hand, and the browser download moves from a postinstall hook to an explicit install-browsers target so a plain install does not fetch Chromium. The Playwright webServer starts the app through nx. clean is cacheable with no outputs. A dependency runs on every invocation whatever the cache state of its dependents, so an uncached clean would delete lib under a running dev server every time anything depended on compile; caching it means it re-runs only when the project's inputs changed, which is exactly when compile re-runs too. The root and browser extension package.json set nx.includedScripts to [] so nx does not infer targets from their scripts; without it the root's `eslint` script became a target that recursively invoked run-many. @aws/pdk is still a runtime dependency of threat-composer-infra (PDKPipeline, pdk-nag) and requires projen as a peer, which eject had removed from the tree, so projen is declared where it is actually needed. Replacing the PDK constructs is a separate change. CI, the Dockerfile, the husky hooks and scripts/build.sh call yarn and nx directly; `yarn build` replaces `pdk run build` plus the separate eslint and ruff steps. docs/ still describe the pdk workflow and are updated when the package manager changes. Verified: `yarn build` from an empty nx cache passes all 39 tasks; a second run hits the cache for 38 and leaves the working tree clean. The Playwright suite passes at 126 passed, 5 skipped, unchanged from main. * build: Move from yarn workspaces to pnpm pnpm-lock.yaml is generated with `pnpm import`, so it carries the versions yarn.lock had resolved rather than re-resolving every caret range; dependency upgrades are a separate change. pnpm 10 is pinned through packageManager and installed directly (npm install -g pnpm@10) rather than through corepack. The yarn resolutions block becomes pnpm.overrides. The two path resolutions, nx/**/minimatch and syncpack/**/minimatch, become the direct-child selectors nx>minimatch and syncpack>minimatch, which is what they matched: both packages depend on minimatch directly. The workspaces field moves to pnpm-workspace.yaml; the wxt nohoist entry has no equivalent and none is needed, since pnpm does not hoist. onlyBuiltDependencies allows esbuild's install script, which the project's build needs; the other packages pnpm reported are listed under ignoredBuiltDependencies so the warning does not recur. pnpm's strict node_modules exposed dependencies that were resolved only through yarn's hoisting: - the app's service worker imports workbox-core, workbox-expiration, workbox-precaching, workbox-routing and workbox-strategies, which came in through react-scripts; now declared at the version react-scripts uses - the app imports types from mdast (@types/mdast) for the docx converter; now declared, on the 3.x line that matches remark-parse 10 - scripts/generateSchema.ts imports zod at the root; now a root devDependency at the library's range - e2e requires @cloudscape-design/components/test-utils/selectors; now declared, pinned to the version the library uses, because those selectors encode Cloudscape's hashed class names and a mismatch would match nothing The app's dependency on the library becomes workspace:*, since pnpm does not link workspace packages by version match. The app's own resolutions block is removed: yarn honours resolutions only at the root, so it had never taken effect. PDKPipeline's default synth step runs `npx projen install` and `npx projen build`, neither of which exists after eject, so the pipeline stack now passes explicit install and build commands (pnpm) and the snapshot is updated to match. The PDK constructs themselves are replaced in a later change. CI, the Dockerfile, the husky hook, scripts/ and the Playwright webServer command call pnpm. Documentation that described the pdk and yarn workflow is rewritten for pnpm and nx. Verified: `pnpm install --frozen-lockfile` from an empty node_modules, then `pnpm build` passes all 39 tasks and leaves the working tree clean. The Playwright suite passes at 126 passed, 5 skipped. * build: Replace eslint-plugin-header with its maintained fork eslint-plugin-header 3.1.1 last shipped in 2022 and has no rule schema, so it cannot load under ESLint 9 or later flat config (Stuk/eslint-plugin-header#56, #59). @tony.ganchev/eslint-plugin-header is a drop-in fork that keeps the legacy array configuration and supports ESLint 7 through 10, which the coming dependency upgrade needs. Only the plugin name in each package's .eslintrc.json changes. header.js is untouched and `pnpm eslint` reports no findings and rewrites no files, so every source banner stays byte-identical to main. Checked in both directions: a file without a banner fails with "missing header", and --fix inserts a banner identical to the existing ones. * build: Move to Vite, Vitest and ESM, and upgrade dependencies Replaces Create React App and craco with Vite, jest with Vitest in every TypeScript package, the Storybook webpack builder with the Vite builder, and moves the packages to ESM. Every npm dependency moves to its current release: React 19, TypeScript 6, ESLint 10, Storybook 10, nx 23, Vite 8, Vitest 5, wxt 0.21, and Node 24 in CI, the Dockerfile and the CodeBuild synth step. Migration and upgrade are one commit because switching the bundler brings in a new dependency set anyway, and regenerating the lockfile re-resolves everything in it once; doing them separately would move most packages twice for no review benefit. The app builds three artefacts from one vite.config.ts, selected by --mode: website, browser-extension and ide-extension. The mode files .env.browser-extension and .env.ide-extension carry VITE_APP_MODE, so they are committed and excluded from the .env ignore. REACT_APP_* becomes VITE_*, PUBLIC_URL becomes Vite's base, the service worker is built with vite-plugin-pwa's injectManifest strategy, and ReactDOM.render becomes createRoot, which React 19 requires and which also removes the React 17 compatibility-mode warning the e2e console guard had to allow. TypeScript is 6.0.3 rather than 7: the library and app need moduleResolution node10 for Cloudscape's type-only subpath imports, which 7 removes, and typescript-eslint and Vite consume the compiler API that 7.0 does not yet expose. ESLint 10 needs flat config; each package's eslint.config.mjs wraps its existing .eslintrc.json through FlatCompat so the rule set does not silently drift, with import replaced by import-x and the header rule shared from .eslintrc.header.json. No consistent-type-imports rule is added, so source files are not rewritten to import type; the only type-only changes are the ones Vite requires. The library's tsconfig enables isolatedModules because Storybook and Vitest now bundle its source directly, and four barrel files that re-exported types with a plain export {} are corrected to export type, which Rollup otherwise reports as a missing export. Cloudscape's internal/* subpaths are absent from its exports map, so those imports become import type, which is what they are and lets import-x/no-unresolved skip them. Typecheck is a build dependency in every package. It had rotted unnoticed: the library's tsconfig included only *.ts, so no .tsx file was ever typechecked, and 16 unused React default imports and an unused import in the extension surface now. Vite strips types without checking them, so the build alone would not catch this. Vitest runs without --updateSnapshot, so the infra pipeline snapshot now fails the build on drift instead of being rewritten silently. It is regenerated here for the Vitest header and the Node 24 runtime. aws-cdk-lib stays at 2.194.0 in infra because @aws/pdk's static-website types do not compile against newer releases; both move in the next change, which replaces the PDK constructs. New nx targets: typecheck everywhere, prepare (wxt prepare) for the extension, preview for the app, and e2e:preview, e2e:variants, e2e:extension and e2e:all for the suites that could not exist before Vite: production preview (6 tests), the two embedded build variants (29) and the packed Chrome extension loaded into Chromium (25), plus 67 Vitest unit tests for the extension's pure logic. The .wxt directory is generated by wxt prepare and is no longer tracked. The root postinstall runs nx with the daemon off, since the daemon restarts when node_modules changes and the postinstall raced it. Security: of the repository's 96 open npm Dependabot alerts, this leaves 3 open, all aws-cdk-lib, which the next change closes. Checked by evaluating each alert's vulnerable_version_range against every version in pnpm-lock.yaml with node-semver; the same check against the previous lockfile reports 96 of 96 still vulnerable, so it is discriminating. 57 close through upgrades and 36 because the package leaves the tree. Two overrides are scoped rather than global: brace-expansion@>=5 (three major lines are live and all patched) and @aws-northstar/ui>uuid (the only consumer still asking for 9). Verified on Node 24 from an empty node_modules: pnpm install --frozen-lockfile, then pnpm build passes all 44 tasks; a second run hits the cache for 43 and leaves the working tree clean apart from schemas/threat-composer-v1.schema.json, which zod 4 emits in a different key order and is committed so the build stays idempotent. Playwright: web app 126 passed, 5 skipped, unchanged from CRA; preview 6, variants 29, extension 25. * infra: Replace the @aws/pdk constructs with plain AWS CDK @aws/pdk is deprecated, depends on projen at runtime (the only reason projen was still in the tree after eject), and its static-website types no longer compile against current aws-cdk-lib, which is what held aws-cdk-lib at 2.194.0 in the previous change. The package now depends only on aws-cdk-lib, cdk-nag and constructs, all at their current releases, and the root package.json loses the CDK dependencies that existed solely to satisfy PDK's peers. PDKNag.app() becomes a plain App with the cdk-nag AwsSolutionsChecks aspect. PDKPipeline and PDKPipelineWithCodeConnection become pipelines.CodePipeline with a CodeBuildStep synth (pnpm install --frozen-lockfile, pnpm build, Node 24), with the nag suppressions PDK applied internally now written out. StaticWebsite becomes src/static-website.ts: a private S3 bucket, a CloudFront distribution with an Origin Access Control, and a BucketDeployment. CloudfrontWebAcl becomes src/web-acl-stack.ts, a plain CfnIPSet and CfnWebACL. Two behavioural differences, both recorded in the package README: the WebACL lives in a companion us-east-1 stack, since CloudFront-scoped WAF resources exist only there and PDK reached that region through a Lambda-backed custom resource, so a stage should be deployed as a whole (cdk deploy 'Dev/*'); and the optional Sonar scanner PDKPipeline could attach is not carried over, since sonarqubeScannerConfig is unset. The pipeline snapshot is regenerated. With aws-cdk-lib at 2.269 the last 3 of the repository's 96 open npm Dependabot alerts close; the check from the previous change now reports 0 still vulnerable. * build: Finish the hooks and restore e2e files to main's version scripts/build.sh ran `pnpm eslint` after `pnpm build`, but every package's build target already depends on its eslint target, so the second run was redundant. The pre-push hook is now install plus build. The husky hooks drop the `#!/usr/bin/env sh` and `husky.sh` lines that husky 9 deprecates and husky 10 will reject. Four e2e files had been taken from the earlier branch, which forked from #332 before its final revision, so they were older than main: tests/preview-and-content.spec.ts and fixtures/cloudscape.ts go back to main's version, and the two hint messages that named scripts which do not exist here point at the nx targets that build the artefacts. * ci: Scope the deploy build job's GITHUB_TOKEN The build job in deploy.yml was the only job in either workflow without an explicit permissions block, which CodeQL reports under actions/missing-workflow-permissions. It only reads the repository, so contents: read is sufficient. Kept at job level because the deploy and release jobs already declare their own wider scopes and a job-level block replaces the workflow default outright. * build: Leave Cloudscape subpath imports as they were The previous change rewrote 18 imports of Cloudscape's internal/* and */interfaces subpaths to `import type`, because import-x/no-unresolved cannot see those modules: they carry types only and are absent from the package's exports map. That is a resolver limitation, not a problem with the source, so the rule now ignores those two path patterns in the library and app configs and the 18 files go back to main's text. TypeScript still resolves the imports through node10 resolution and erases them on emit, as it always did. * build: Strip the husky hooks to their commands The pre-commit and pre-push hooks carried explanatory comments; the review asked for them to go. Each hook is now the single command it runs. * infra: Install uv in the CodeBuild synth step The synth step runs pnpm install and pnpm build. The root postinstall runs uv sync for packages/threat-composer-ai, and the build graph runs its ruff and pytest targets, so uv has to be present. CI and the Dockerfile already install it; CodeBuild did not. pip3 puts the binary on the PATH that later buildspec phases inherit, where a curl install into ~/.local/bin would not. Snapshot regenerated. The infra README's deploy examples also named stacks that do not exist (ThreatComposerApplicationStack, ThreatComposerPipelineStack); they now use the real ids, ThreatComposerInfraStack and the Dev/* stage, which is also what the migration note below them says to do. * build: Drop changes that were not needed Three e2e files (fixtures/routes.ts and two serve-*.mjs scripts) and the two data scripts under scripts/data plus scripts/generateSchema.ts had been rewritten by the earlier branch (blank-line removal, node: import prefixes, __dirname replaced with import.meta.url). None of it is required: tsx runs main's versions unchanged and produces identical output, so the files go back to main's text apart from the usage comments, which named ts-node. scripts/exportExamples.ts uses top-level await and the library it loads is now ESM, so it cannot stay CommonJS; tsx decides the module format from the nearest package.json, which declares none, so the file is renamed .mts to make the format explicit. Verified by running it. .nxignore is restored with the projen marker removed, which is what eject left behind. * build: Keep types.ts and configs/features.ts as files types.ts gains declarations for .svg, .jpg and side-effect .css imports, which TypeScript 6 now requires (TS2882), but it did not need to become a .d.ts: a .ts file holding only ambient module declarations compiles under isolatedModules, so it keeps its name and the change is a modification rather than a rename. configs/features.ts is a banner-only placeholder that nothing imports; deleting it was not part of the migration. * build: Run cdk through pnpm exec in the deploy scripts yarn workspace <pkg> run <name> fell through to the package's binaries when no script matched, so 'run cdk deploy' worked. pnpm run does not (ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT), and the infra package declares no cdk script because its targets live in project.json. Use pnpm exec, which runs the binary in the package directory where cdk.json lives. Also correct two comments in the e2e preview suite that named an app script that does not exist; the build is an nx target. * build: Keep a stray .env virtualenv out of the Docker image and ruff A Python virtualenv named .env in packages/threat-composer-ai was copied into the image because .dockerignore only excluded .venv. With .git also excluded, ruff cannot honour .gitignore inside the image, so it linted the whole virtualenv and the build failed. Exclude .env and .ruff_cache in .dockerignore and add .env to ruff's exclude list beside .venv. The app's .env.browser-extension and .env.ide-extension are different names and are unaffected. * build: Include e2e lint in the build and simplify pre-push * fix: Preserve infrastructure and correct build caching --------- Co-authored-by: Darran Boyd <drboyd@example.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ESLint 9 now requires that plugins provide a schema if they accept options; see https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#changes-to-how-you-write-rules.