-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: upgrade to TypeScript 7 #4318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import { defineConfig } from "tsdown"; | ||
|
|
||
| export default defineConfig({ | ||
| entry: ["src/index.ts"], | ||
| format: ["cjs", "esm"], | ||
| fixedExtension: false, | ||
| tsconfig: "tsconfig.src.json", | ||
| dts: true, | ||
| sourcemap: true, | ||
| clean: true, | ||
| treeshake: true, | ||
| minify: false, | ||
| deps: { | ||
| onlyBundle: false, | ||
| alwaysBundle: [ | ||
| // Always bundle internal packages | ||
| /^@internal/, | ||
| // Always bundle ESM-only packages | ||
| "nanoid", | ||
| "p-limit", | ||
| ], | ||
| dts: { | ||
| // The TypeScript 7 declaration emitter consumes referenced package declarations. | ||
| neverBundle: [/^@internal/], | ||
| }, | ||
| }, | ||
| // rolldown injects its own `__require` helper in the ESM output as | ||
| // `createRequire(import.meta.url)` with no fallback. When this ESM bundle is | ||
| // re-bundled into a CJS consumer (e.g. the webapp server), esbuild replaces | ||
| // `import.meta.url` with `undefined`, so `createRequire(undefined)` throws at | ||
| // startup. Patch the helper to fall back to a valid path, matching the | ||
| // fallback the previous tsup banner provided. | ||
| plugins: [ | ||
| { | ||
| name: "resilient-create-require", | ||
| renderChunk(code: string) { | ||
| if (!code.includes("createRequire(import.meta.url)")) return null; | ||
| return { | ||
| code: code.replaceAll( | ||
| "createRequire(import.meta.url)", | ||
| "createRequire(import.meta.url || process.cwd() + '/index.js')" | ||
| ), | ||
| map: null, | ||
| }; | ||
| }, | ||
| }, | ||
| ], | ||
| }); | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,19 @@ | ||
| import { spawnSync } from "node:child_process"; | ||
| import { createRequire } from "node:module"; | ||
| import { | ||
| cpSync, | ||
| existsSync, | ||
| mkdirSync, | ||
| mkdtempSync, | ||
| readFileSync, | ||
| realpathSync, | ||
| rmSync, | ||
| symlinkSync, | ||
| writeFileSync, | ||
| } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join, resolve } from "node:path"; | ||
| import { dirname, join, resolve } from "node:path"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import ts from "typescript"; | ||
|
|
||
| /** | ||
| * Regression test for declaration-emit portability (customer TS2742). | ||
|
|
@@ -76,40 +79,39 @@ describe("declaration emit portability", () => { | |
| } | ||
|
|
||
| const fixturePath = join(consumerDir, "agent.ts"); | ||
| ts.sys.writeFile(fixturePath, FIXTURE_SOURCE); | ||
|
|
||
| const emitted = new Map<string, string>(); | ||
| const host = ts.createCompilerHost({}); | ||
| host.writeFile = (fileName, text) => emitted.set(fileName, text); | ||
|
|
||
| const program = ts.createProgram({ | ||
| rootNames: [fixturePath], | ||
| options: { | ||
| target: ts.ScriptTarget.ES2022, | ||
| module: ts.ModuleKind.NodeNext, | ||
| moduleResolution: ts.ModuleResolutionKind.NodeNext, | ||
| strict: true, | ||
| declaration: true, | ||
| emitDeclarationOnly: true, | ||
| skipLibCheck: true, | ||
| outDir: join(consumerDir, "out"), | ||
| rootDir: consumerDir, | ||
| }, | ||
| host, | ||
| }); | ||
| writeFileSync(fixturePath, FIXTURE_SOURCE); | ||
| writeFileSync( | ||
| join(consumerDir, "tsconfig.json"), | ||
| JSON.stringify({ | ||
| compilerOptions: { | ||
| target: "ES2022", | ||
| module: "NodeNext", | ||
| moduleResolution: "NodeNext", | ||
| strict: true, | ||
| declaration: true, | ||
| emitDeclarationOnly: true, | ||
| skipLibCheck: true, | ||
| outDir: "out", | ||
| rootDir: ".", | ||
| }, | ||
| files: ["agent.ts"], | ||
| }) | ||
| ); | ||
|
|
||
| const emitResult = program.emit(); | ||
| const diagnostics = [ | ||
| ...ts.getPreEmitDiagnostics(program), | ||
| ...emitResult.diagnostics, | ||
| ].filter((d) => d.category === ts.DiagnosticCategory.Error); | ||
| const formatted = diagnostics.map((d) => | ||
| ts.flattenDiagnosticMessageText(d.messageText, "\n") | ||
| const require = createRequire(import.meta.url); | ||
| const typescriptRoot = dirname(require.resolve("typescript/package.json")); | ||
| const result = spawnSync( | ||
| process.execPath, | ||
| [join(typescriptRoot, "bin", "tsc"), "-p", consumerDir], | ||
| { | ||
| encoding: "utf8", | ||
| } | ||
| ); | ||
|
carderne marked this conversation as resolved.
|
||
| expect(formatted).toEqual([]); | ||
| const compilerOutput = [result.stdout, result.stderr].filter(Boolean).join("\n"); | ||
| expect(result.error, compilerOutput).toBeUndefined(); | ||
| expect(result.status, compilerOutput).toBe(0); | ||
|
|
||
| const dts = [...emitted.entries()].find(([name]) => name.endsWith("agent.d.ts"))?.[1]; | ||
| expect(dts).toBeDefined(); | ||
| const dts = readFileSync(join(consumerDir, "out", "agent.d.ts"), "utf8"); | ||
|
Comment on lines
+103
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 declaration-emit test now shells out to the TS CLI instead of the in-process API The test was rewritten to write a Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| // The payload generic must be named via the public subpath, and the | ||
| // emit must not fall back to file paths into the package. | ||
| expect(dts).toContain('import("@trigger.dev/sdk/chat").ChatTaskWirePayload'); | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 redis-worker published .d.ts may now reference private @internal packages
The old tsup config bundled
@internal/*into both the JS output AND the declaration output (noExternal: [/^@internal/]+dts: true, where rollup-plugin-dts inlines those types). The new tsdown config keeps JS bundling (deps.alwaysBundle: [/^@internal/]) but explicitly disables it for declarations (deps.dts.neverBundle: [/^@internal/]atpackages/redis-worker/tsdown.config.ts:22-25). The public surface of@trigger.dev/redis-workerleaks@internal/redistypes (Redis,RedisOptionsinpackages/redis-worker/src/worker.ts:1) and@internal/tracingtypes (Span,Tracer,Meterinpackages/redis-worker/src/fair-queue/types.ts:1-3). Since@internal/*are private workspace packages (devDependencies, never published), a publisheddist/index.d.tsthat references them viaimport("@internal/redis")would be unresolvable for any external npm consumer, regressing type portability that the old build provided. The inline comment indicates this is a deliberate TS7-driven choice, so it may be intentional / acceptable if the package is effectively internal-only — worth confirming whether external consumers rely on these types resolving.Was this helpful? React with 👍 or 👎 to provide feedback.