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(tray): 🩹 Fixes the context loaded async in the tray. #2593

Merged
merged 4 commits into from
Aug 22, 2023

Conversation

whizzzkid
Copy link
Contributor

@whizzzkid whizzzkid commented Aug 22, 2023

Fixes: #2577

In this PR:

  • Passing the missing context in tray methods.
  • This is a bandage fix to push out a working build.
  • prolly release this as v0.30.1

TODO: Move this to better type-definitions to not run into this.

@whizzzkid whizzzkid requested review from SgtPooki and a team as code owners August 22, 2023 04:58
@@ -33,7 +33,7 @@
// "typeRoots": [], /* Specify multiple folders that act like `./node_modules/@types`. */
"types": ["node", "./types/countly-sdk-nodejs"], /* Specify type package names to be included without being referenced in a source file. */
// "allowUmdGlobalAccess": true, /* Allow accessing UMD globals from modules. */
// "resolveJsonModule": true, /* Enable importing .json files */
"resolveJsonModule": true, /* Enable importing .json files */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was a simple fix to require('../../package.json') errors.

Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

i've got some changes local.. just gonna test now

src/tray.js Outdated
Comment on lines 163 to 171
click: () => { runGarbageCollector(ctx) },
enabled: false
},
{ type: 'separator' },
{
id: 'moveRepositoryLocation',
label: i18n.t('moveRepositoryLocation'),
click: () => { moveRepositoryLocation() }
// @ts-ignore
click: () => { moveRepositoryLocation(ctx) }
Copy link
Member

Choose a reason for hiding this comment

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

This won't work because ctx is different now (need to use ctx.get('getIpfsd')). We should be able to use getCtx() in runGarbageCollector and moveRepositoryLocation, or simply pass the ctx methods required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, my bad, it seemed to work but mostly would've been buggy. I wonder what's a good way to test this 🤔

Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

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

@whizzzkid take a peek at these new changes when you get a chance, and then let's merge and release when you're good with it

@whizzzkid whizzzkid merged commit 3e35523 into main Aug 22, 2023
7 checks passed
@whizzzkid whizzzkid deleted the fix/lazy-load-app-context branch August 22, 2023 21:20
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.

[IPFS Desktop] TypeError: Cannot destructure property 'stopIpfs' of 'undefined' as it is undefined.
2 participants