icp-cli: bindgen record fields with opt are undefined when empty, not null

Đang mở Phù hợp với người mới
  • Có gì không ổn?
#409 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 6 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
78/100
Loại issue
Tài liệu
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
typescript
Lĩnh vực
documentation, testing

Hướng nghiên cứu

Đọc skills/icp-cli/references/binding-generation.md:67-69 và eval hiện có tên là “Candid opt T representation in bindgen”. Cập nhật tài liệu để phân biệt các giá trị opt đơn thuần với các trường record tùy chọn, khuyến nghị cách kiểm tra giá trị rỗng không phụ thuộc phiên bản và thêm một eval cho trường hợp trường record. Công việc hoàn tất khi mapping đã chỉnh sửa được ghi lại trong tài liệu và eval mới chạy thành công.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

skills/icp-cli/references/binding-generation.md:67-69 says bindgen's wrapper turns opt T into T | null and tells agents to "always use T | null". That holds for a plain opt, but not for a record field: an opt T field is an optional property that is undefined when empty. An agent following the rule writes if (r.value !== null), which is true for an empty field. The certified-variables helper had exactly this bug: it rejected a valid proof of absence (fixed in #407).

Verified against the @icp-sdk/bindgen 0.4.0 and 0.4.1 wrappers:

Candid position Wrapper type Empty value
argument, return value, vec element, variant payload: opt T T | null null
record field f : opt T f?: T undefined (field omitted)
nested opt opt T (not in a record) Some<T | null> | None None

Needed regardless of the bindgen PRs

These hold on 0.4.1 and after dfinity/icp-js-bindgen#197, dfinity/icp-js-bindgen#198 and dfinity/icp-js-bindgen#201:

  • Correct the section in binding-generation.md. T | null applies to a plain opt. A record field is optional and undefined when empty. To send an absent field, omit it or pass undefined.
  • Recommend == null or ?? for "is it empty". It is correct in every position and version, so an agent doesn't need to know which position it is in.
  • Add an eval for the record-field case. The existing case ("Candid opt T representation in bindgen") covers only a plain return value, and its expected result !== null stays correct.
  • Fix it in place, in icp-cli. #406 moves binding-generation.md's opt section into canister-calls later; the move then carries the corrected text. #406's own Candid↔TS table had the same gap (opt T → T | null only) and is corrected there, since it is the long-term home of the full mapping.

After the bindgen fixes are released

  • dfinity/icp-js-bindgen#197 (present but falsy optionals): until it ships, 0, false and "" in an optional record field or variant payload are sent as absent. No skill change is needed once it ships. If it is skipped, add a pitfall: send such values only in a plain opt argument, or avoid falsy values in optional fields.

  • dfinity/icp-js-bindgen#198 (opt opt record fields): once released, document f?: T | null with its three states (omitted, null for opt null, a value), pinned to that bindgen version. If it is skipped, the skill must say instead that opt opt record fields do not match their declared type (they decode to Some/None objects, and null cannot be sent), and point to the raw declarations for those fields.

  • dfinity/icp-js-bindgen#201 (actor class name): once released, generating the actor files for a .did file named after one of its own types (governance.did with type Governance, swap.did with type Swap) fails with a hint to rename the file. Where the skill shows vendoring NNS or SNS interfaces, if anywhere, use a distinct file name (e.g. nns_governance.did). The same advice applies if it is skipped, since the class then merges with the type without an error.

  • dfinity/icp-js-bindgen#203 (docs for --actor-interface-file): no skill change. It documents the .d.ts as a reference that leaves out the actor class, which is how caffeine-app (--actor-interface-file) and writing-motoko ("use the generated backend.d.ts as the source of truth") already use it; neither imports it.

  • dfinity/icp-js-bindgen#172 (proposed, no PR yet): if adopted, an inline unit variant becomes a string-literal union instead of a Variant_<tags> or borrowed enum. No skill references Variant_<tags> today, so nothing breaks; the mapping in #406 then gains that row.

Until the bindgen PRs are decided, document only the first section.

Ngôn ngữ chính
Python
Star
39
Fork
19
Merge trung bình
5 ngày 3 giờ
Pull request đã merge (30 ngày)
19

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của dfinity/icskills

Tất cả issue của dfinity/icskills

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.