fix: generated declarations fail with skipLibCheck disabled by GiHoon1123 · Pull Request #227 · toss/es-git

GiHoon1123 · 2026-09-21T11:29:15Z

Summary

Fix generated declarations for consumers using skipLibCheck: false without dropping iterator helper types.

Use the exported Reference type in getAnnotatedCommitFromReference.
Declare generated iterators as classes merged with IteratorObject interfaces.
Typecheck ES2022 and ESNext consumer examples in CI.

Closes #215

vercel · 2026-09-21T11:29:21Z

@GiHoon1123 is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

Use the exported Reference type in generated signatures and declare iterator classes with the portable IterableIterator shape. Add a consumer typecheck fixture so the declarations are checked with skipLibCheck disabled.

marshallku

I reproduced the original problem and confirmed the fix. With main's index.d.ts, a consumer with skipLibCheck: false gets 11 × TS2689, plus TS2552 (GitReference) and TS2488, and this branch compiles cleanly. The bindings are in sync with a fresh build, and the transform is idempotent.

However, the iterator change regresses existing consumers, so this needs another pass before merge. Details and a tested alternative are inline.

Severity: P1 must fix before merge · P2 should fix before merge · P3 worth fixing · P4 suggestion.

marshallku · 2026-10-08T06:59:52Z

+      node.superTypeParameters = null;
+      node.implements = [
+        ...(node.implements ?? []),
+        j.tsExpressionWithTypeArguments(j.identifier('IterableIterator'), j.tsTypeParameterInstantiation([yieldType])),


P1 · compatibility — Swapping extends Iterator<T, void, void> for implements IterableIterator<T> removes the iterator helper typings from every #[napi(iterator)] class, even though the runtime still has them. A consumer with lib: ["ESNext"] doing repo.branches().map(b => b.name).toArray() compiles against main and fails on this branch with TS2339: Property 'map' does not exist on type 'Branches'. The removed doc text explicitly promised these helpers, so this is a breaking type change shipping as a fix:.

An alternative that fixes both cases (I hand-edited Branches and checked the ES2022/skipLibCheck: false consumer and an ESNext .map().toArray() consumer): a class declaration merged with an interface built on IteratorObject:

export declare class Branches { next(value?: void): IteratorResult<BranchesItem, void> [Symbol.iterator](): Branches } export interface Branches extends IteratorObject<BranchesItem, void, void> {}

IteratorObject is in lib.es2015.iterable (TS ≥ 5.6), and esnext.iterator adds the helpers to it. This does not raise the minimum TS version, because extends Iterator on main already needs 5.6. It also lets the class comment keep its link to the helper docs.

(Severity: one reviewer rated this P1, the other P2.) (found independently by two reviewers)

marshallku · 2026-10-08T06:59:52Z

+
+      for (const comment of path.parent.node.comments ?? []) {
+        comment.value = comment.value.replace(
+          /\n \* This type extends JavaScript's `Iterator`, and so has the iterator helper\n \* methods\. It may extend the upcoming TypeScript `Iterator` class in the future\.\n \*\n \* @see https:\/\/developer\.mozilla\.org\/en-US\/docs\/Web\/JavaScript\/Reference\/Global_Objects\/Iterator#iterator_helper_methods\n \* @see https:\/\/www\.typescriptlang\.org\/docs\/handbook\/release-notes\/typescript-5-6\.html#iterator-helper-methods/,


P3 · robustness — This regex matches napi-rs's boilerplate comment word for word. If a napi-rs upgrade changes the wording, nothing fails: the stale "extends Iterator" text just stays in the published docs. Consider matching more loosely, for example any comment containing This type extends JavaScript's `Iterator` , or throwing when a transformed class's comment wasn't rewritten.

marshallku · 2026-10-08T06:59:52Z

+    .find(j.ClassDeclaration, node => node.superClass?.type === 'Identifier' && node.superClass.name === 'Iterator')
+    .forEach(path => {
+      const node = path.node;
+      const yieldType = node.superTypeParameters?.params[0] ?? j.tsAnyKeyword();


P4 · robustness — If the heritage clause ever has no type parameters, yieldType quietly becomes any and the generated types get looser without anyone noticing. Throwing a descriptive error here would surface an unexpected napi-rs output shape at build time instead.

marshallku · 2026-10-08T06:59:52Z

@@ -0,0 +1,12 @@
+{
+  "compilerOptions": {
+    "target": "ES2022",


P3 · tests — This fixture only covers an ES2022 consumer, which is why the helper regression above passes CI. A second config with "lib": ["ESNext"] whose consumer calls something like repo.branches().map(b => b.name).toArray() would lock in the helper typings too.

marshallku · 2026-10-08T06:59:52Z

  pub fn get_annotated_commit_from_reference(
    &self,
-    reference: &GitReference,
+    reference: &crate::reference::Reference,


P3 · docs — Two things around this signature:

docs/reference/Repository/Methods/getAnnotatedCommitFromReference.md:17 and its docs/ko counterpart still show the non-existent GitReference. just docs-gen reference --pattern='Repository/Methods/getAnnotatedCommitFromReference' fixes the English page; the Korean one needs the same one-word change.

The full path works because napi-rs prints the type exactly as written in the signature. Without a note, someone could reintroduce a use … as alias and bring the bug back. A one-line comment would prevent that.

Optional, in the same doc block (L102/L112, not from this PR): "Creates a Annotated Commit" → "Creates an Annotated Commit", and "Reference to creates a Annotated Commit." → "Reference to create an Annotated Commit from." (found independently by two reviewers)

GiHoon1123 · 2026-10-08T07:52:50Z

Thanks, this caught a real regression. I switched the generated iterators to the class + IteratorObject form and added an ESNext consumer check for .map().toArray(). The transform now fails on unexpected type arguments or helper comments, and I corrected the Reference docs in both languages. yarn test:types and yarn biome check pass locally.

marshallku

LGTM! Thanks for your contribution

Behavior changes for existing `Repository#commit()` callers: - Invalid explicit `author` or `committer` identities now throw instead of silently falling back to the repository default. - Unsigned commits now preserve `timeOptions.offset` instead of writing `+0000`, so the same inputs can produce a different OID. - Signed commits remove exactly one trailing LF from the supplied signature, so raw GPG output can produce a different OID than in 0.7.x. Closes toss#219 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The merge-base changed after approval.

GiHoon1123 requested review from marshallku and seokju-na as code owners September 21, 2026 11:29

fix: generated declarations fail with skipLibCheck disabled

1030f63

Use the exported Reference type in generated signatures and declare iterator classes with the portable IterableIterator shape. Add a consumer typecheck fixture so the declarations are checked with skipLibCheck disabled.

GiHoon1123 force-pushed the fix/skiplibcheck-declaration-errors branch from 0cebbc6 to 1030f63 Compare September 21, 2026 23:32

marshallku reviewed Oct 8, 2026

View reviewed changes

GiHoon1123 added 5 commits October 8, 2026 16:40

fix: retain iterator helper types in declarations

6d71ba2

fix: reject incomplete generated iterator types

f707470

test: cover iterator helpers in ESNext consumers

1275cf4

docs: correct annotated commit reference type

a4c236d

fix: keep iterator helper comments in sync

3a32322

Merge branch 'main' into fix/skiplibcheck-declaration-errors

f57731b

marshallku previously approved these changes Oct 8, 2026

View reviewed changes

poketopa and others added 2 commits October 11, 2026 15:45

Merge branch 'main' into fix/skiplibcheck-declaration-errors

cdd7a99

marshallku force-pushed the main branch from 93bbfe8 to f3602d2 Compare October 11, 2026 06:47

marshallku merged commit 83524a9 into toss:main Oct 11, 2026
37 of 38 checks passed

Navigation Menu

Repository navigation

fix: generated declarations fail with skipLibCheck disabled - #227

fix: generated declarations fail with skipLibCheck disabled#227
marshallku merged 9 commits into
toss:mainfrom
GiHoon1123:fix/skiplibcheck-declaration-errors

GiHoon1123 commented Sep 21, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Sep 21, 2026

Uh oh!

marshallku left a comment

Uh oh!

marshallku Oct 8, 2026

Uh oh!

marshallku Oct 8, 2026

Uh oh!

marshallku Oct 8, 2026

Uh oh!

marshallku Oct 8, 2026

Uh oh!

marshallku Oct 8, 2026

Uh oh!

GiHoon1123 commented Oct 8, 2026

Uh oh!

marshallku left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

GiHoon1123 commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

vercel Bot commented Sep 21, 2026

Uh oh!

marshallku left a comment

Choose a reason for hiding this comment

Uh oh!

marshallku Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

marshallku Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

marshallku Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

marshallku Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

marshallku Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

GiHoon1123 commented Oct 8, 2026

Uh oh!

marshallku left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GiHoon1123 commented Sep 21, 2026 •

edited

Loading