Conversation
🦋 Changeset detectedLatest commit: 8726025 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
| "eslint-plugin-jest": "^29.2.1", | ||
| "eslint-plugin-react-hooks": "^7.0.1", | ||
| "globals": "^16.5.0", | ||
| "typescript-eslint": "^8.49.0", |
There was a problem hiding this comment.
@trevor-cortex Is any of this required in the scope of this PR?
There was a problem hiding this comment.
Good question. The @solana/eslint-config-solana bump from v5 to v6 is not strictly required for the TypeScript v6 upgrade itself. The TS 6.0 changes (tsconfig updates) are independent of the ESLint config version.
However, it's likely included because the v5 eslint config may have compatibility issues with TS 6.0 (e.g. typescript-eslint needing a version that supports TS 6.0's new module resolution semantics). If pnpm lint passes on the main branch with TS 6.0 + the old eslint config v5, then this bump could be split into a separate PR.
The new explicit deps (eslint-plugin-jest, eslint-plugin-react-hooks, globals, typescript-eslint) are peer dependencies of @solana/eslint-config-solana v6 — with pnpm's strict dependency resolution, they need to be listed explicitly. They wouldn't be needed if you stayed on v5.
So: if you want to keep this PR minimal (just TS 6.0 compat), you could try keeping @solana/eslint-config-solana at v5 and only bump it if linting breaks. That would remove all 6 of these dependency changes and leave just the typescript version bump.
There was a problem hiding this comment.
@trevor-cortex Hmm why would bumping @solana/eslint-config-solana from v5 to v6 make use install these internal dependencies? Can you double check your findings?
There was a problem hiding this comment.
You're right to push back — I was wrong about these being "internal dependencies that now need to be explicit."
Looking at the lockfile, @solana/eslint-config-solana v6.0.0 declares all of these as peer dependencies:
eslint-plugin-jest: ^29.2.1eslint-plugin-react-hooks: ^7.0.1globals: ^16.5.0typescript-eslint: ^8.49.0
(along with @eslint/js, eslint, eslint-plugin-simple-import-sort, eslint-plugin-sort-keys-fix, eslint-plugin-typescript-sort-keys, jest, and typescript)
With pnpm's strict dependency model, peer deps need to be explicitly listed by the consumer. But this isn't a change from v5 → v6 that introduced these as new peer deps. More likely, v5 had a similar set of peer deps, and the previous package.json was relying on pnpm's autoInstallPeers: true (which is set in the lockfile) to resolve them transitively without listing them.
So the real question is: why are these 4 now listed explicitly when they weren't before? It could be that v5 had them as regular dependencies (not peer deps), or that v6 added them as new peer deps. Without checking v5's package.json I can't say for certain, but my earlier claim that this was about pnpm strictness was an oversimplification.
Bottom line: if you want to test whether the eslint config bump is needed at all, try keeping v5 and running pnpm lint with TS 6.0. If it passes, you can drop all 6 dependency changes from this PR.
There was a problem hiding this comment.
@ioxde Sorry for the AI noise but I think these should go. Lmk if I'm missing something.
There was a problem hiding this comment.
Done! No you didn't miss anything. I reverted the unnecessary package updates. Do you want me to merge the commits into one and force push?
There was a problem hiding this comment.
After reinstalling with the d52663e change eslint complained about being unsupported. I pushed 8726025 as the minimal amount of changes I could make to get rid of that warning.
pnpm lint
> @codama/renderers-js@2.1.0
> eslint . && prettier --check .
=============
WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
* @typescript-eslint/typescript-estree version: 8.43.0
* Supported TypeScript versions: >=4.8.4 <6.0.0
* Your TypeScript version: 6.0.2
Please only submit bug reports when using the officially supported version.
=============
Checking formatting...
All matched files use Prettier code style!After 8726025:
pnpm lint
> @codama/renderers-js@2.1.0 lint
> eslint . && prettier --check .
Checking formatting...
All matched files use Prettier code style!See also:
anza-xyz/eslint-config-solana@22456fd
typescript-eslint/typescript-eslint@4933417
|
@trevor-cortex Can you please review this PR and explain to me why (or if) the changes are important in the context of a TypeScript v6 upgrade (whilst keeping v7 in mind). |
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
This PR upgrades TypeScript from v5.9 to v6.0 and updates the ESLint toolchain to match. The tsconfig changes are minimal and well-targeted. Here's a breakdown of each change and why it matters:
tsconfig.json changes
1. moduleResolution: "node" → moduleResolution: "nodenext" (+ module: "nodenext")
This is the critical fix. moduleResolution: "node" (aka node10) is deprecated in TS 6.0 and will be removed in TS 7.0. It encoded Node.js 10-era resolution behaviour and doesn't reflect how modern Node resolves modules (e.g. exports maps, ESM support). nodenext is the correct choice for a library that targets Node.js directly (as opposed to bundler for bundled apps). Adding explicit module: "nodenext" is the right pairing.
2. Removed esModuleInterop: true
In TS 6.0, esModuleInterop defaults to true and setting it to false is deprecated. Since it was already true here, removing it is just config cleanup — no behavioural change.
3. Added "types": ["node"]
TS 6.0 changed the types default from "all @types packages" to an empty array. Without this explicit setting, @types/node wouldn't be picked up anymore, causing errors like Cannot find name 'process'. This is a required migration step.
4. target stays "ESNext"
No change needed here since the project was already on ESNext (the new default is es2025, but ESNext >= es2025 so no issue).
tsconfig.declarations.json change
5. Added rootDir: "./src"
TS 6.0 changed the rootDir default from "inferred common directory of input files" to the tsconfig.json directory (i.e. .). Without this explicit rootDir, declaration files would emit to dist/types/src/index.d.ts instead of dist/types/index.d.ts. Since this config emits declarations with outDir: "./dist/types", setting rootDir: "./src" preserves the expected output structure. Good that it's only on the declarations config since the base tsconfig.json doesn't emit.
Dependency updates
6. @solana/eslint-config-solana v5 → v6 + new peer deps
The eslint config package bumped its peer requirements to match TS 6.0 compatibility. The new direct deps (eslint-plugin-jest, eslint-plugin-react-hooks, globals, typescript-eslint) were previously transitive or provided by the config — now they need to be explicit peers. This is just keeping the ESLint toolchain compatible with TS 6.0.
TS 7.0 readiness
All the deprecated options that were addressed here (moduleResolution node10, implicit rootDir, implicit types) will become hard errors in TS 7.0 (the Go-based rewrite). This PR properly addresses them rather than using ignoreDeprecations: "6.0", which is the right call.
Looks clean. The tsconfig changes are the minimum necessary set for TS 6.0 compatibility, and they're all forward-compatible with 7.0. The PR description mentions generated code may need testing in a followup — that's the right scope boundary for this.
|
Re: The build script is With explicit
Without this change, TS 6.0's new default ( |
|
Re: removing Additionally, explicitly setting |
Trimmed down version of #135 with only the necessary changes for project-level types. Generated code might need to be tested in a followup commit.
"moduleResolution": "node"is deprecated in 6.0 and removed in 7.0.