chore(js-sdk,cli): modernize tsconfig and bump TypeScript to 6.0#1516
chore(js-sdk,cli): modernize tsconfig and bump TypeScript to 6.0#1516mishushakov wants to merge 1 commit into
Conversation
Now that the bundler (tsup) owns emit and `tsc` is type-check only, the compiler options can be modernized: - bump `typescript` to `^6.0.3` in both packages - js-sdk: target/lib `es6`->`es2022`, add `module: esnext`, `moduleResolution` `node`->`bundler`; drop `allowJs` (no JS sources) and `allowSyntheticDefaultImports` (implied by `esModuleInterop`) - cli: `moduleResolution` `node`->`bundler`; drop strict-implied flags (strictNullChecks, strictFunctionTypes, strictBindCallApply, strictPropertyInitialization, noImplicitThis, alwaysStrict) and the deprecated `downlevelIteration` (no-op at es2022) - cli: replace TS6-deprecated `baseUrl` with explicit `paths`, and drop `outDir` (unused under `tsc --noEmit`; it triggered a TS6 rootDir check against js-sdk source pulled in via the `e2b` path mapping) - js-sdk: add `ignoreDeprecations: "6.0"` to silence the deprecated `baseUrl` that tsup's d.ts builder hardcodes; removable once we move to tsdown Target stays at es2022 (still on Node 20). No public API or runtime change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 78bf701 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
PR SummaryLow Risk Overview For js-sdk, compile target/lib move from For CLI, Patch changesets for both packages; lockfile updated. No application source or public API changes intended. Reviewed by Cursor Bugbot for commit 78bf701. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from eb54914. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.31.1-mishushakov-modernize-sdk-tsconfig.0.tgzCLI ( npm install ./e2b-cli-2.13.1-mishushakov-modernize-sdk-tsconfig.0.tgzPython SDK ( pip install ./e2b-2.30.0+mishushakov.modernize.sdk.tsconfig-py3-none-any.whl |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78bf7014f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "declaration": true, | ||
| // tsup's d.ts builder always injects `baseUrl` (deprecated in TS 6.0). This | ||
| // silences that deprecation; remove once we migrate off tsup (tsdown). | ||
| "ignoreDeprecations": "6.0", |
There was a problem hiding this comment.
Add Node types when migrating to TS6
With TypeScript 6, compilerOptions.types defaults to [], so installing @types/node is no longer enough for this config to see Node globals/built-ins. pnpm run typecheck/pnpm build will hit TS2591/TS2503 on existing usages such as process/NodeJS.Timeout in packages/js-sdk/src/template/logger.ts and Buffer in packages/js-sdk/src/envd/rpc.ts; the CLI config changed in this commit has the same problem for its tests. Add "types": ["node"] to the affected tsconfigs instead of only suppressing TS6 deprecations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Build-only change with thorough verification and no bugs found, but the TypeScript 5→6 major bump plus the moduleResolution: bundler switch and the deprecation workarounds (ignoreDeprecations, outDir removal, baseUrl→paths remap) are non-trivial — worth a human eye on the build pipeline.
Extended reasoning...
Overview
Touches only build configuration for two packages: packages/js-sdk and packages/cli. Bumps typescript from ^5.4.5 to ^6.0.3, switches moduleResolution to bundler, raises js-sdk target/lib to es2022/es2022, removes options that are now redundant under strict / esModuleInterop or deprecated in TS 6.0, and remaps the CLI's src/src/* bare imports through paths to replace the removed baseUrl. A changeset entry (patch for both packages) and the pnpm lockfile update accompany the change. No source code or public API is modified.
Security risks
None meaningful. Configuration of a build/typecheck toolchain doesn't change runtime behavior, and the published artifacts are still emitted by tsup with its own config. No auth, crypto, network, or permission code is involved.
Level of scrutiny
Medium — higher than a typical config tweak. It's a build-only change so blast radius is bounded, but it is a major TypeScript version bump (5 → 6) accompanied by several non-obvious workarounds the author flagged themselves: ignoreDeprecations: "6.0" silences a tsup-internal deprecation, outDir is removed from the CLI tsconfig to dodge a new TS 6 rootDir inference (TS6059), and the CLI's baseUrl-based bare imports are remapped through paths to keep from 'src/...' resolving. The switch to moduleResolution: bundler also changes how tsc resolves imports during type-check / dts generation. Any one of these is reasonable; together they merit a human eye on the build pipeline.
Other factors
The author reports successful typecheck, build (including .d.ts), lint, and that pnpm test produces an identical pass/fail breakdown to the pristine tree (failures are pre-existing API-key-missing failures). The CLI binary was smoke-tested with --version. The bug-hunting system found no issues, and the cursor bot summary characterizes the change as low risk. The changeset is correctly scoped as a patch for both packages. The combination of "thoroughly verified" plus "major toolchain bump with caveats the author flagged" is exactly the case where a human reviewer adds the most value — the verification is strong but the surface area of behavior changes in TS 6 is broad enough that I'd rather a maintainer sign off.
What & why
Now that the bundler (tsup) owns the actual emit and
tscis used only for type-checking /.d.tsgeneration, the compiler options can be modernized. Also bumps TypeScript across both packages.Internal build-config change only — no public API or runtime behavior changes. Verified that
pnpm testoutput is byte-identical before/after (the failures present in this workspace are pre-existing and purely from a missingE2B_API_KEY).Compiler options: before → after
packages/js-sdk/tsconfig.jsontargetes6es2022lib["dom","ESNext"]["dom","es2022"]moduleesnextmoduleResolutionnodebundlerallowJstrue.jssources)allowSyntheticDefaultImportstrueesModuleInterop)ignoreDeprecations"6.0"(added — see note)packages/cli/tsconfig.jsonmoduleResolutionnodebundlerstrictNullChecks,strictFunctionTypes,strictBindCallApply,strictPropertyInitialization,noImplicitThis,alwaysStricttruestrict: true)downlevelIterationtruees2022)baseUrl"."paths{ e2b }{ src, "src/*", e2b }(replacesbaseUrlfor the existingsrc/...import style)outDir"dist"target/libfor the CLI were alreadyes2022, so they're unchanged.TypeScript
Both packages:
typescript^5.4.5→^6.0.3.Decisions & behavior-affecting notes
Target stayed at
es2022, notes2023. The repo is still on Node 20 (.tool-versions=nodejs 20.19.5,engines >= 20,@types/node ^20); the Node 22 migration hasn't landed, so I used the agreedes2022fallback.moduleResolution: "bundler"is the headline risk. Both packages typecheck and build cleanly under it (module: es2022/esnextare both compatible withbundler). For the CLI, the previousbaseUrl-based bare imports (import … from 'src/user',from 'src') are preserved by mapping them throughpaths; the bundled output still resolves them (build verified, CLI binary smoke-tested).TS 6.0 fallout (because we picked TS 6 over "latest 5.x"):
baseUrlanddownlevelIterationare deprecated for removal in TS 7.0; removed both.outDirremoval: undertsc --noEmit, TS 6.0 now validates the inferredrootDiragainst all program files. Thee2b→../js-sdk/srcpath mapping pulls js-sdk source into the CLI program, which lives outsidepackages/cli, producingTS6059. RemovingoutDir(unused by--noEmit; tsup has its own output config) stopsrootDirinference and clears the error with no build impact.ignoreDeprecations: "6.0": tsup 8.4.0's.d.tsbuilder hardcodes a (now-deprecated)baseUrl(baseUrl: compilerOptions.baseUrl || "."), which fails the DTS build on TS 6.0. There's no way to stop the injection from config, so this silences it. It's commented in the file and should be removed once we migrate to tsdown (which is also why the original plan preferred basing this on the tsdown branch).Not done (intentionally)
verbatimModuleSyntax— would require 177import typeconversions (78 js-sdk + 99 cli, allTS1484), spreading this config PR across many source files. Left out as excessive churn; can be a follow-up.tsconfig.base.json— the two configs share only ~8 options and diverge on many more (lib,module,paths,rootDirs,isolatedModules,preserveSymlinks, etc.), so it doesn't factor out cleanly. Skipped to avoid forcing it.Verification
pnpm run typecheck✅ both packagespnpm run build✅ both packages (js-sdk ESM + CJS + DTS; cli CJS; binary smoke-tested with--version)pnpm run lint✅ both packages (oxlint)pnpm run format— no changespnpm run test— neutral: identical pass/fail to the pristine tree (237 failed | 127 passed | 1 skipped, all failures from missingE2B_API_KEYin this workspace; pure-logic unit tests pass)🤖 Generated with Claude Code