chore: await initialization before routing a msg #2364
Conversation
This uses a deferred promise to make sure the initialization has finished before we process a message call.
|
🚀 Thanks for the pull request! Here are the current build files for testing: Download and unzip the file for your browser. Refer to the readme for detailed install instructions. This build is brought to you by: Want to sponsor the next build? send some sats to ⚡️builds@getalby.com (don't forget to provide your name) Don't forget: keep earning sats! |
| utils.openUrl("welcome.html"); | ||
| } | ||
| if (isRecentlyUpdated) { | ||
| migrate(); |
There was a problem hiding this comment.
Should we consider moving migrate() to the bottom of the init() function? it is also an asynchronous function which I believe should run before the init finishes?
There was a problem hiding this comment.
You probably mean to the top of the init function, right? For me it would make sense to await the migration and only then resolveInit to avoid calls being executed while migrations are still taking place.
There was a problem hiding this comment.
I agree resolveInit shouldn't be called until the migrations have run. But don't the migrations depend on init? that's why I suggested it being moved to the last line in the init() function.
|
@bumi the fix works well for me, for a basic test of adding a 10 second sleep to the start of the init function. Without the fix doing this does some strange things, like launching the welcome screen and asking to set a new password. |
|
Did some testing and added delays to test. Routing calls are stacked during initialization and resolved after that: 👍️
|
chore: also wait for migrations before finishing init
* master: fix: change link color to blue-600 / blue-700 on hover (#2372) feat: lnurlpay screen toggle more fields (#2373) chore: yarn lock fix: wait for transactions to load fix: use account id from connector directly fix: tabs hover / dark mode (#2360) feat: add account-context balanceLoading (#2359) yarn lock file bug: show no txs msg after loading bug: show no txs msg after loading
* origin/await-init: fix: only log migration message if recently updated chore: re-add isRecentlyUpdated check chore: also wait for migrations before finishing init
Immediatlly set the connector to make sure a second getConnector() call returns the same connector instance.
LNC initializes a wasm instance which also has some callbacks and writes those in a global variable based on the namespace. Without the namespace the wasm varable gets reused and the credentials get overwritten. https://github.com/lightninglabs/lnc-web/blob/ af1604c26295b86191d8f97a71a3f6cab6433812/lib/lnc.ts#L74-L76
Just in case there is any react dependency on it
|
New dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No new dependency issues detected in pull request Bot CommandsTo ignore an alert, reply with a comment starting with Pull request alert summary
📊 Modified Dependency Overview:
|
| await msg.request("selectAccount", { | ||
| id: accountId, | ||
| }); | ||
| setAccountId(accountId); |
There was a problem hiding this comment.
Why did you change the order here?
This leads to some side effects (e.g. the avatar changes only after the account has been loaded) 🤔
There was a problem hiding this comment.
is that a problem?
I wanted to avoid some react dependency to do requests before the new account was switched.
This uses a deferred promise to make sure the initialization has finished before we process a message call.
This also fixes some LNC issues and makes sure that the WASM is namespaced.