Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(dts/ffi): non-exact types break FFI inference #14968

Merged
merged 3 commits into from
Jun 27, 2022
Merged

fix(dts/ffi): non-exact types break FFI inference #14968

merged 3 commits into from
Jun 27, 2022

Conversation

sno2
Copy link
Contributor

@sno2 sno2 commented Jun 26, 2022

Closes #14963

Notes:

  • The tuple item mapping method is more efficient than recursion and supports as many iterations as a union can have: ~50,000 parameters instead of the 1,000 that regular tail-end recursion optimized (which the other implementation was not) generics.
  • The FooMap and Foo<Key> method was previously encouraged by Ryan Kavanaugh and it allows TypeScript to inline some types and for composition between things that may want to also check if something is Foo via FooMap.

@sno2
Copy link
Contributor Author

sno2 commented Jun 26, 2022

I'm not sure how to run the linter on the file. It passes deno lint but CI seems to be failing.

Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint can be run using eg.

deno run --unstable --allow-run=git,./third_party/prebuilt/linux64/dlint,cargo --allow-read=./ --allow-write=./ ./tools/lint.js --check

Aside from that, the tests should be moved to the dedicated test file, FromNativeTypeMap needs to be fixed and some changes seem unnecessary though I don't know for sure.

cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
cli/dts/lib.deno.unstable.d.ts Outdated Show resolved Hide resolved
@sno2 sno2 requested a review from littledivy June 27, 2022 01:48
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@littledivy littledivy merged commit 89fc240 into denoland:main Jun 27, 2022
@sno2 sno2 deleted the fix/ffi-types branch June 27, 2022 12:47
dsherret pushed a commit to dsherret/deno that referenced this pull request Jun 30, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TS error on FFI imported functions since release 1.23.1
3 participants