-
-
Notifications
You must be signed in to change notification settings - Fork 50
don't generate unused functions in bindings #138
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?
Conversation
once: ( | ||
cb: TAURI_API_EVENT.EventCallback<T>, | ||
) => ReturnType<typeof TAURI_API_EVENT.once<T>>; | ||
emit: T extends null |
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.
Can you explain this change?
It also differs from the JS Doc alternative so they will need to match for this to be merged.
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.
to be honest i don't know when/how this changed. i will undo the change.
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.
For some reason when running tests, it creates this change. Thought I'd broken something somewhere but it just happens when you run tests lol
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.
Okay cool. As long as the JS doc part also matches we can resolve this.
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.
Asked for some clarifications
I commented and made a small change. Is there anything else that is still wrong? |
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.
Sorry I haven't had a chance to come back to this.
I think some major changes have landed on main
to the way the JS bindings are written so this PR has some conflicts. Would be awesome to get them resolved and we can merge this!
|
||
import { | ||
invoke as TAURI_INVOKE, | ||
Channel as TAURI_CHANNEL, |
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.
I think this is reqiured when using channels for them to work. They must be called Channel
as we basically rely on shadowing to make it work.
once: ( | ||
cb: TAURI_API_EVENT.EventCallback<T>, | ||
) => ReturnType<typeof TAURI_API_EVENT.once<T>>; | ||
emit: T extends null |
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.
Okay cool. As long as the JS doc part also matches we can resolve this.
When there are no events, part of the bindings file is unused.
In my case with
"noUnusedLocals": true,
in the tsconfig this lead to my project not compiling.This PR splits the global.ts and global.js into two parts and only includes the event related code if there are any events defined.
I tested this in my project, but I am not very used to ts/js. So feel free to point out any issues.