When storage or folder is removed, Detail components should render without error

[hikerpig]

Description

Edit a note and unlink its folder or storage, current implementation of browser/main/Detail/MarkdownNoteDetail.js will throw exception due to invalid object property access, this PR add a safe guard to prevent it.

Issue fixed

• Adding a Storage that you have unlinked once throws an error when unlinking it again #2876

Type of changes

• 🔘 Bug fix (Change that fixed an issue)
• ⚪ Breaking change (Change that can cause existing functionality to change)
• ⚪ Improvement (Change that improves the code. Maybe performance or development improvement)
• ⚪ Feature (Change that adds new functionality)
• ⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

• 🔘 My code follows the project code style
• ⚪ I have written test for my code and it has been tested
• 🔘 All existing tests have been passed
• ⚪ I have attached a screenshot/video to visualize my change if possible

[ZeroX-DG]

@hikerpig can you resolve the conflict please?

[hikerpig]

@ZeroX-DG Done 🎉

[ZeroX-DG]

@hikerpig thank you for resolving the conflict. It seem the bug is still there, I haven't have the time to look into it yet, if you can investigate this bug before me please do:)

[hikerpig]

@ZeroX-DG I see many features are deployed after my initial commit. All this errors occur when no storage matches the storageKey, some of them are purposely thrown by this findStorage method.

I just added some guards to avoid some scenarios, currently my editor won't break after storage is unlinked.

But I see there are still some places called findStorage without a try/catch guard, maybe a thorough mine-sweeping is needed?

[ZeroX-DG]

@hikerpig yes, I check out some places that call findStorage and I think it's not very safe to not catch the error at all. But catching the error is a difficult work, you have to choose to ignore an action or show a dialog to the user so if you want we can separate it into another PR.

[hikerpig]

That will be great. This PR should be done by now.

And I think instead of throwing an error in findStorage, it should just return null if no matches found, and all callers should keep in mind and handle the null result, a simple if (storage) test would be better than try/catch. How do you feel about this @ZeroX-DG ?

If that's ok, I will take a look in this and open a draft PR in one or two days.

[ZeroX-DG]

@hikerpig that's actually a good suggestion since findStorage only throw 1 error, we can reduce that to return null instead. BTW, I think the bug is not quite fixed yet, after you perform all the steps to reproduce the bug, everything works ok untill you click on the tag list button on side bar or when you add and unlink the storage another time. The new bug is caused by this function:

https://github.com/BoostIO/Boostnote/blob/5f56d3e0de48e3c49b454558caac5c81ea4430cf/browser/main/NoteList/index.js#L1108

I think it's caused by inconsistent between note list and storage list since the storage has been removed but the note list still try to access it. Can you investigate and fix it also? Thank you 👍

[hikerpig]

@ZeroX-DG I've fixed the errors in NodeList/index.js, have a look ~

[ZeroX-DG]

Very cool. LGTM 🎉