Cleanup JS api types #3131

jpatters · 2023-12-14T16:40:50Z

Description

The adds better type support for the js apis. It also makes the exports from the npm package more conventional. Previously usage looked like this:

import { Dialogs } from "@wailsapp/api";

Dialogs.Dialog.Info({Message: "hello world"});

usage now looks like this

import { Dialog } from "@wailsapp/api";

Dialog.Info({Message: "hello world"});

Because of the import change, I have marked this as a breaking change however, the documentation in the readme for the js package already described it as working this way even though it did not.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using wails doctor.

Windows
macOS
Linux

Test Configuration

Please paste the output of wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.

Checklist:

I have updated website/src/pages/changelog.mdx with details of this PR
My code follows the general coding style of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

…} from "@wailsapp/api"

netlify · 2023-12-14T16:40:53Z

‼️ Deploy request for dapper-pastelito-9b5e5b rejected.

Name	Link
🔨 Latest commit	`df01313`

coderabbitai · 2023-12-14T16:40:58Z

Important

Auto Review Skipped

Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

You can reply to a review comment made by CodeRabbit.
You can tag CodeRabbit on specific lines of code or files in the PR by tagging @coderabbitai in a comment.
You can tag @coderabbitai in a PR comment and ask one-off questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

leaanthony · 2023-12-15T04:04:51Z

v3/internal/runtime/desktop/api/window.js

 */
 Center: () => void wails.Window.Center(),
 /**
 * Set the window title.
- * @param title
+ * @param {string} title
+ * @returns {Promise<void>}


I believe void will mean nothing is returned?

I believe the function that it calls also returns Promise<void>. (https://github.com/jpatters/wails/blob/js-api-types/v3/internal/runtime/desktop/window.js#L65).

And when I followed it up the chain, it ended up here: https://github.com/jpatters/wails/blob/js-api-types/v3/internal/runtime/desktop/runtime.js#L84

Is this incorrect?

Yeah the call to the backend returns a promise but the void here ignores it and returns nothing at all.

Ok. Happy to update. What is the desired behaviour? I would have expected a promise.

There's nothing to wait for as there's nothing being returned so it works like a normal synchronous function I guess.

I see. Is there an opportunity for the request to the backend to fail though? Should the promise propagate up?

I don't believe so but we could make promise a standard regardless. Just had PRs in the past to remove that when they don't return anything.

Yeah. If there's no opportunity for the request from the backend to be rejected then that makes sense for sure. I'll dig through some more code to see what might happen on the backend and come back with a stronger opinion.

As you rightly said, those wrappers are out of date so they very well may have changed.

So, the backend does return http errors when things go wrong and the response handler does properly call reject when the http code is not in the 200 range. So I would suggest not swallowing those errors and allowing the promise to propagate up to the caller. It will require a few more changes for this to all behave as expected as the method that this calls also swallows the rejection. But minor changes. If this sounds reasonable, I will make the changes in this pr.

leaanthony · 2023-12-15T09:19:53Z

Please could you update the changelog located at mkdocs-website/docs/changelog.md? The one in the PR is for v2, Thanks!

jpatters · 2023-12-15T12:04:48Z

Please could you update the changelog located at mkdocs-website/docs/changelog.md? The one in the PR is for v2, Thanks!

Ah. Good to know. Thanks.

…acted to

leaanthony · 2024-01-03T08:47:42Z

@jpatters - A lot has gone on since this PR was open. Let's discuss on discord 🙏

jpatters · 2024-01-05T13:04:25Z

@jpatters - A lot has gone on since this PR was open. Let's discuss on discord 🙏

Yeah, for sure

jpatters added 4 commits December 14, 2023 12:00

add missing jsdoc so we can have types

81e8343

build types as part of build

fd5b060

make exports a little more conventional. this allows import { Dialog …

c6f3d89

…} from "@wailsapp/api"

since we have types now we don't need to call them out in the readme

ab82b8c

leaanthony reviewed Dec 15, 2023

View reviewed changes

update changelog

df01313

jpatters force-pushed the js-api-types branch from 9ea5b29 to df01313 Compare December 16, 2023 12:07

leaanthony added the On Hold label Dec 17, 2023

jpatters added 3 commits December 20, 2023 09:23

consistency. consistent types, returns, and function calls

f5be0a0

return http errors so that they can be caught on the front end and re…

0e98cd4

…acted to

needed to add WailsEvent to the window and rebuild

020fe73

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup JS api types #3131

Cleanup JS api types #3131

jpatters commented Dec 14, 2023

netlify bot commented Dec 14, 2023 •

edited

coderabbitai bot commented Dec 14, 2023

Auto Review Skipped

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

leaanthony Dec 15, 2023

jpatters Dec 15, 2023

leaanthony Dec 15, 2023

jpatters Dec 15, 2023

leaanthony Dec 15, 2023

jpatters Dec 15, 2023

leaanthony Dec 15, 2023

jpatters Dec 15, 2023

leaanthony Dec 15, 2023

jpatters Dec 18, 2023

leaanthony commented Dec 15, 2023

jpatters commented Dec 15, 2023

leaanthony commented Jan 3, 2024

jpatters commented Jan 5, 2024

Cleanup JS api types #3131

Are you sure you want to change the base?

Cleanup JS api types #3131

Conversation

jpatters commented Dec 14, 2023

Description

Type of change

How Has This Been Tested?

Test Configuration

Checklist:

netlify bot commented Dec 14, 2023 • edited

‼️ Deploy request for dapper-pastelito-9b5e5b rejected.

coderabbitai bot commented Dec 14, 2023

Auto Review Skipped

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leaanthony commented Dec 15, 2023

jpatters commented Dec 15, 2023

leaanthony commented Jan 3, 2024

jpatters commented Jan 5, 2024

netlify bot commented Dec 14, 2023 •

edited

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)