Fix/persisted native message system #11039
Merged
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
vytick
reviewed
Feb 5, 2024
| const isAnyOfMessageSystemAffectingActions = isAnyOf(messageSystemActions.fetchSuccessUpdate); | ||
| const isAnyOfMessageSystemAffectingActions = isAnyOf( | ||
| messageSystemActions.fetchSuccessUpdate, | ||
| deviceActions.selectDevice, |
There was a problem hiding this comment.
It seems, that there are also other actions that maybe should trigger the check - deviceChange, disconnect, forget, auth. Wdyt?
There was a problem hiding this comment.
- deviceChanged and auth are not relevant (no relevant change for available message system conditions can be updated)
- disconnect and forget would also trigger selectDevice if selected, but not relevant if not selected
There was a problem hiding this comment.
but definitelly I'm glad you asked 👍 ❤️
matejkriz
reviewed
Feb 5, 2024
There was a problem hiding this comment.
FIXED in 1b5794b
Have you found a way to test it? I've tried to test it locally with custom local config (simulating remote fetch failed) and I'm now facing this error:
ERROR [Error: Unable to resolve module ./suite-common/message-system/files/config.v1 from /Users/matejkriz/Projects/trezor-suite/suite-native/app/.:
None of these files exist:
* suite-common/message-system/files/config.v1(.android.ts|.native.ts|.ts|.android.tsx|.native.tsx|.tsx|.android.mjs|.native.mjs|.mjs|.android.js|.native.js|.js|.android.jsx|.native.jsx|.jsx|.android.json|.native.json|.json|.android.cjs|.native.cjs|.cjs|.android.scss|.native.scss|.scss|.android.sass|.native.sass|.sass|.android.css|.native.css|.css)
But I believe it should work... #7936 (comment)
|
@tomasklim please check 1b5794b I know it's not great, but hopefully not terrible? |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
matejkriz
approved these changes
Feb 7, 2024
|
/rebase |
- Conditional import for some reason start using wrong base path on mobile. Regular import on top of the file fix the issue. - Potential downside is that local config is fully bundled in the code so it can increase bundle size on web.
trezor-ci
force-pushed
the
fix/persisted-native-message-system
branch
from
1b5794b to
c200b55
Compare
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.
Description
Related Issue
Resolve #10979