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: blank screen after 2.0.5 #604

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Conversation

shaiu
Copy link
Contributor

@shaiu shaiu commented Oct 11, 2024

This fixes the issue in 2.0.5 when loading the app there is a blank screen (no importers and exporters)

Added a default config to ConfigStore constructor

@baruchiro
Copy link
Collaborator

I think it will be fixed by #601, if not, I will merge yours.

@shaiu
Copy link
Contributor Author

shaiu commented Oct 13, 2024

I think it will be fixed by #601, if not, I will merge yours.

How exactly? I think I introduced a major bug in 2.0.5 that erases users config..
So this should be merged and also we should remove 2.0.5 from Releases

@baruchiro
Copy link
Collaborator

I think it will be fixed by #601, if not, I will merge yours.

How exactly? I think I introduced a major bug in 2.0.5 that erases users config.. So this should be merged and also we should remove 2.0.5 from Releases

Do you think it is a bug introduced in 2.0.5?
If so, we didn't find it, we just covered it, and it may return. You and my solutions both fall back to the default config (in my fix I'm doing it right after reading the config, if it is an empty object...). If the bug is still there, users may be reset to the default config without warning.

Anyway, I pushed my fix, and if the bug is still there, we will see it happen and find it.

@shaiu shaiu closed this Oct 13, 2024
@shaiu shaiu reopened this Oct 14, 2024
@baruchiro
Copy link
Collaborator

@shaiu why did you open it again?

I think it can be closed.

@shaiu
Copy link
Contributor Author

shaiu commented Oct 14, 2024

@baruchiro
I believe that this is the bug:
config: Config = {} as Config;

@baruchiro
Copy link
Collaborator

Oh, I see it now.

But I prefer you don't create a default config here. We already have the default config on the 'main'/'backend' side, and the saveConfigIntoFile will not save it if it is undefined. Then, it will try to load it from the backend, and if it is empty or does not exist on the filesystem, the backend will return the default config.

@baruchiro baruchiro self-requested a review October 14, 2024 10:11
@baruchiro baruchiro merged commit 3f036a6 into brafdlog:master Oct 14, 2024
8 checks passed
Copy link
Contributor

🎉 This PR is included in version 2.1.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@shaiu shaiu deleted the shaiu-fix-blank branch October 14, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants