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 broken feature layout due to async race condition on getRpcDriver #2551

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Nov 24, 2021

This appears to fix #2547

The summary of #2547 is that multiple rpc drivers were being created due to the async change of getDriver

This changes it back to not being an async call, but loses the storing of ipc call to userData with this

If it is needed to have userData for other purposes in the code, it could be obtained in another way?

Alternatively, we could modify this PR to try to avoid double-creation of the drivers e.g. with a promise-singleton type pattern seen elsewhere in the codebase

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Nov 24, 2021
@cmdcolin cmdcolin added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) bug Something isn't working labels Nov 24, 2021
@codecov
Copy link

codecov bot commented Nov 24, 2021

Codecov Report

Merging #2551 (0df5bce) into main (4952829) will increase coverage by 0.00%.
The diff coverage is 33.33%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2551   +/-   ##
=======================================
  Coverage   60.69%   60.70%           
=======================================
  Files         547      547           
  Lines       25582    25575    -7     
  Branches     6042     6041    -1     
=======================================
- Hits        15528    15526    -2     
+ Misses       9728     9723    -5     
  Partials      326      326           
Impacted Files Coverage Δ
packages/core/rpc/WebWorkerRpcDriver.ts 0.00% <ø> (ø)
...ckages/development-tools/src/createRollupConfig.ts 0.00% <0.00%> (ø)
packages/core/rpc/RpcManager.ts 87.09% <66.66%> (+4.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update beb3643...0df5bce. Read the comment docs.

Copy link
Collaborator

@garrettjstevens garrettjstevens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a version of plugin loading that saved downloaded plugin code to the userData directory, but it doesn't do that anymore, so it should be fine to get rid of this leftover bit. We can probably get rid of this handler in electron.ts, too:

ipcMain.handle('userData', () => {
return userData
})

Good catch!

@cmdcolin cmdcolin merged commit b90f78b into main Nov 25, 2021
@cmdcolin
Copy link
Collaborator Author

might be ok to hold onto the vestigial IPC call there...maybe it will find another use :)

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.

Broken feature rendered
2 participants