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

Fix timing issue with electron@8 #65

Merged
merged 3 commits into from
May 12, 2020
Merged

Conversation

steveetm
Copy link
Contributor

Fix issue #64
ipc.connect() is an async function, I don't really know why it is working with electron@7, but it is definitely not a good idea to expect client to be ready to send messages without waiting connec()

@mrfelton
Copy link

Tested ACK

@veado
Copy link

veado commented Apr 27, 2020

Have checked that ^ in a project running on Electron 8.x - it works great! Thx @steveetm for this fix!

@AndreyBelym
Copy link
Contributor

Thank you for your contribution. In the older (>5.0.0) Electron version there was no documentation on the webContents.loadURL return type:
https://github.com/electron/electron/blob/v4.2.12/docs/api/web-contents.md.

Since it was unclear if this function has some synchronous side effects, I purposely tried to keep the loadURL mock synchronous to avoid breaking the user's app behavior.

If we want to make loadURL asynchronous, I will need some time to check if it works in the older Electron versions and doesn't introduce unwanted side effects.

@veado
Copy link

veado commented Apr 28, 2020

If we want to make loadURL asynchronous, I will need some time to check if it works in the older Electron versions and doesn't introduce unwanted side effects.

@AndreyBelym How about to stick on v0.0.15-alpha.1 or < v0.0.14 for using older Electron versions ^, but not blocking this PR?

@AlexKamaev
Copy link
Contributor

@veado
We do not block this PR. We just need time to research it deeply.

@AndreyBelym AndreyBelym merged commit 73eebb1 into DevExpress:master May 12, 2020
@mrfelton mrfelton mentioned this pull request Jun 6, 2020
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

Successfully merging this pull request may close these issues.

6 participants