Skip to content
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

SolidJS implementation has faults #31

Open
Rafferty97 opened this issue Aug 11, 2024 · 0 comments
Open

SolidJS implementation has faults #31

Rafferty97 opened this issue Aug 11, 2024 · 0 comments

Comments

@Rafferty97
Copy link

Hi, I'm seeking to use something like this in my own Tauri 2 application which uses Solid.JS.

While things work fairly well, I did notice that the maximize button did not change properly as the window was being maximized and unmaximized.

Looking at the code, I saw a number of issues I thought were worth addressing:

  • Dependencies on @tauri-apps/* needed to be updated. Not sure on the best policy here, given we're in "pre-release things breaking all the time" territory.
  • The Tauri APIs being used such as plugin-os/type and window/getCurrent now return values not promises, which breaks the existing code.
  • Importing @tauri-apps/api asynchronously seems unnecessarily complicated. Is it for code splitting? If so, this doesn't seem like an appropriate code splitting point given that we would want correct window controls to appear immediately and seamlessly as soon as the application opens
  • The code in plugin-os.ts seems a little overcomplicated and I'm not sure it works as intended. For example, it asserts that if osTypePromise == null then osType must have a value, but osTypePromise is never cleared after it resolves.
  • In plugin-window.ts, createEffect and onCleanup are being used at the top-level which is not correct as they will not work outside a reactive root. This leads to console warnings, and is generally just more complicated than it has to be.
  • The getCurrentWindow method doens't return undefined, providing more opportunity to simplify code, remove ?. operators, etc.

I will submit a PR soon with my proposed changes. I expect some push back so please let me know your concerns or questions about the changes, and if there's anything I've misunderstood above (like the rationale for loading @tauri-apps/api asyncronously) please let me know so I can amend the code and add a relevant explanatory comment.

Rafferty97 added a commit to Rafferty97/tauri-controls that referenced this issue Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant