-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
ui: redesign followups 8 #5445
Merged
Merged
ui: redesign followups 8 #5445
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's no need to have `socket...` and `appSocket...` actions. I did this initially due to a misunderstanding about the sequence of handling from middleware to reducers.
Mainly bumping to get latest `redux-remember`. A change to socket.io required a change to the types in `useSocketIO`.
- Add an error handler to `redux-remember` config using our logger - Add custom errors representing storage set and get failures - Update storage driver to raise these accordingly - wrap method to clear idbkeyval storage and tidy its logic up
This simply logs every action and a diff of the state change. Due to the noise this creates, it's not added by default at all. Add it to the middlewares if you want to use it.
A recent change to ROARR introduced limits to the size of data that will logged. This ends up making our logs far less useful. Change the serializer back to what it was previously.
The previous diff library would present deleted things as `undefined`. Unfortunately, a JSON.stringify cycle will strip those values out. The ROARR logger does this and so the diffs end up being a lot less useful, not showing removed keys. The new diff library uses a different format for the delta that serializes nicely.
- All persisted slices must now have a slice config, consisting of their initial state and a migrate callback. The migrate callback is very simple for now, with no type safety. It adds missing properties to the state. A future enhancement might be to model the each slice's state with e.g. zod and have proper validation and types. - Persisted slices now have a `_version` property - The migrate callback is called inside `redux-remember`'s `unserialize` handler. I couldn't figure out a good way to put this into the reducer and do logging (reducers should have no side effects). Also I ran into a weird race condition that I couldn't figure out. And finally, the typings are tricky. This works for now. - `generationSlice` and `canvasSlice` both need migrations for the new aspect ratio setup, this has been added - Stuff related to persistence has been moved in to `store.ts` for simplicity
Prevents hotkeys from being captured when embeddings are still loading.
psychedelicious
requested review from
blessedcoolant,
maryhipp and
hipsterusername
as code owners
January 8, 2024 13:13
maryhipp
approved these changes
Jan 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Have you discussed this change with the InvokeAI team?
Have you updated all relevant documentation?
Description
Minor fixes/enhancements:
roarr
error message serializer (fixes new issue w/ truncated contexts in console)redux-remember
persist/rehydrate actions (better visibliity when there is a problem)Related Tickets & Documents
QA Instructions, Screenshots, Recordings
The most important thing to test is that upgrades do not crash the UI. Note that this fix only works for v3.5.1 to v3.6.0 and onward. This will not fix other occurrences of the issue when updating from earlier version.
To reproduce the issue:
main
You should get an error:
TypeError: Cannot read properties of null (reading 'id')
Then, to test this PR:
main
, check out this PR branchYou should get no error and the app loads fine.
Merge Plan
This PR can be merged when approved. Please reproduce the crash and fix as described above before approving.