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

API changes tracking issue #28

Open
mischnic opened this issue May 16, 2018 · 11 comments
Open

API changes tracking issue #28

mischnic opened this issue May 16, 2018 · 11 comments

Comments

@mischnic
Copy link
Contributor

mischnic commented May 16, 2018

libui-node libui-napi
libui.Ui.onShouldQuit  libui.onShouldQuit
 UiSlider::constructor() = constructor(0, 100) done
 the free methods  missing (tolerable?!)
 the atomic bool in Windows event-loop not needed anymore

Missing

  • Dialogs
  • Font
  • main/quit
  • uiTimer done (does setTimeout/Interval need patching? NO)

Otherwise, proton-native seems to work.

@parro-it
Copy link
Owner

parro-it commented May 17, 2018

Otherwise, proton-native seems to work.

🎉 we almost complete everything in two weeks
first commit: f90ec05

I'd never have said we can did it at such pace... we should open a start-up and conquer the world 😆

@parro-it
Copy link
Owner

the free methods

Honestly I don't see how they can be useful in a GC language...
We can implements them as no-op just to avoid breaking something in the wild? But I doubt there's something in the wild using these...

@parro-it
Copy link
Owner

libui.Ui.onShouldQuit

Can we put that method on both places, like we did for enums?

@mischnic
Copy link
Contributor Author

I'd never have said we can did it at such pace... we should open a start-up and conquer the world 😆

😆

But I doubt there's something in the wild using these...

👍

Can we put that method on both places, like we did for enums?

👍

@parro-it
Copy link
Owner

I think to avoid to implement quit and main method: they cause node asynchronous functions to stop working

@parro-it
Copy link
Owner

I think libui.Ui.[dialogs] was libui.UiDialogs, as it is now

@mischnic
Copy link
Contributor Author

mischnic commented May 17, 2018

I think libui.Ui.[dialogs] was libui.UiDialogs, as it is now

You're right.

I think to avoid to implement quit and main method: they cause node asynchronous functions to stop working

So the only way to quit an app is to call stopLoop (is that equivalent) ?

@parro-it
Copy link
Owner

parro-it commented May 17, 2018

So the only way to quit an app is to call stopLoop ?

Yes, it internally calls quit(). Calling quit() after startLoop does not stop the app, because there is a libuv handler created just for the purpose of keeping the node event loop running...
stopLoop close that handle before calling quit()

@mischnic
Copy link
Contributor Author

mischnic commented May 17, 2018

I think to avoid to implement quit and main method: they cause node asynchronous functions to stop working

They were also never documented, so it's fine.

@mischnic
Copy link
Contributor Author

Could the eventloop start/stop promise behaviour cause any difference?

@parro-it
Copy link
Owner

Could the eventloop start/stop promise behaviour cause any difference?

Yes, in the previous version, calling two startLoop sequentially, or two stopLoop sequentially, or stopLoop before startLoop is completed, doesn't throw an error.

But what really happened on such pattern greatly depends on the machine speed, current load etc. Probably a segfault or a thread deadlock or other weird things.

In the correct usage pattern, the new implementation should behave like the previous version, and the user is not required to await the promise, if he don't plan to start a loop immediately after the stop promises is settled (or viceversa)

So, I don't know if this could be considered a breaking change. It's basically a breaking change of behaviour in case of an error condition... I guess this could be fine

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

2 participants