[$8000] The message got sent but it also stayed in the compose box

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:

Type something
Press the send icon
Right after pressing the send icon, start typing again quickly without any break. The compose box will still have the text you just sent

Expected Result:
Compose box should clear
Actual Result:
Compose box doesn't clear
Workaround:
unknown
Platform:

Where is this issue occurring?

iOS
Android

Version Number: 1.1.85-8
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
related to https://github.com/Expensify/App/issues/4129
https://user-images.githubusercontent.com/43995119/181589365-7a29867b-c0cd-412f-ac2a-2db3d0c42cb8.MP4
Expensify/Expensify Issue URL:
Issue reported by: @mountiny
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1658727800463809
View all open jobs on GitHub

I was able to reproduce on my phone, so assigning external.

I think its duplicate of https://github.com/Expensify/App/issues/4129

I think its duplicate of https://github.com/Expensify/App/issues/4129

@mateusbra Thanks! We are aware of that, it is mentioned in the issue body. That issue got very long and hard to follow so we decided to make a clean start and just refer to it. Lots of time has passed since then so maybe easier and cleaner solutions will pop up.

This https://github.com/Expensify/App/issues/8592#issuecomment-1152926071 proposal I have explained why this happens and also provided a solution that can solve this issue, as well as another issue, of blue opacity issue remains in iOS after spell check https://github.com/Expensify/App/issues/8592

Getting on it today

Internal: https://www.upwork.com/ab/applicants/1554462059426074624/job-details
External: https://www.upwork.com/jobs/~0130f26b3941d0c325

actually this issue I have discussed here https://github.com/Expensify/App/issues/8592#issuecomment-1152926071 before, but I have not reported this bug on slack and also I did write a proposal there in order to solve
Currently, I'm waiting for the internal team here https://github.com/Expensify/App/issues/8592#issuecomment-1203057240 they decide to split mWeb and iOS as 2 separate issues once I get to know what's next step I will notify you here as well
CC @rushatgabhane

@mountiny  If it's still open, I can resolve it. Kindly let me know.

Proposal

When we add any message which is a typo (misspelled) into the composer and it sends as a message, but also it remains in text input with the auto-correct word in the iOS (app)

Solution

To fix the issue in iOS (App) we need to remove the line setNativeProps in submit() function as we are already clearing the text from the composer after submitting text so applying selection with setNativeProps to empty text causes this issue

Also setNativeProps is deprecated only because it causes weird issues like this one and also it ignores the latest style
// this.textInput.setNativeProps({selection: {start: 0, end: 0}});

https://github.com/Expensify/App/blob/11a1f0debac2dfee461e6b71751c3b3bcfb9f2cf/src/pages/home/report/ReportActionCompose.js#L425-L430
Like this :
 this.props.onSubmit(trimmedComment);
 this.updateComment(''); // here we are already clearing composer textinput so applying 0 selection below this has no effect
 this.setTextInputShouldClear(true);
 // Important to reset the selection on Submit action
 //this.textInput.setNativeProps({selection: {start: 0, end: 0}}); // comment this line solves problem in iOS (app)

Note: commenting on this line won't affect any other platform as we are already clearing the composer
Result (After commenting setNativeProps line) :
Device: iPhone 13(App)
https://user-images.githubusercontent.com/47522946/173188941-a898f924-0cd1-455b-97f5-35d35ee38f0f.mp4
Result (Without commenting setNativeProps line Current behavior) :
https://user-images.githubusercontent.com/47522946/173188991-d0c0ecf2-dcbc-4ea5-b165-69431e46cc8f.mp4
cc @rushatgabhane  this is the same proposal I posted in another issue as this proposal solves 2 issue's of iOS (app)

One easy solution is that, as before entering the message the value for that input is empty string, and after submitting the message if we set the that value to empty string again by using the hooks, it can resolve the issue.
It's one of the easiest solution we can do.

after submitting the message if we set the that value to empty string

@hamzashakeel348 I'm sure we already do that
https://github.com/Expensify/App/blob/1d06ec9cdb650351d4fb3ca51026dec879975090/src/pages/home/report/ReportActionCompose.js#L440
@swiftpipe we avoid workarounds that use setTimeout. It suggests some race condition and doesn't fix the root cause

@rushatgabhane I checked it again. We are doing
` submitForm(e) {
if (e) {
e.preventDefault();
}
    const comment = this.prepareCommentAndResetComposer();
    if (!comment) {
        return;
    }

    this.props.onSubmit(comment);
}`

We are setting props as comment if comment is not empty, we also have to do to make them empty string after submitting data.

@dhairyasenjaliya i'm sorry i don't see how your proposal is related to this issue. Please see the video attached in the issue body for a better understanding

It's same will update video

@rushatgabhane updated the video here https://github.com/Expensify/App/issues/10148#issuecomment-1206003090
Note : In order to understand this issue please follow like this  iOS app

go to any chat
type missed spell word that is actually highlighted by spell checker
wait for spellcheck to turn into blue color word
now send immediately with the blue color word
you will notice blue color text will remain in the composer even after sending a message

also am I eligible to report this issue since I already logged this issue on 10th June here on slack but
somehow at that time it does not log into GitHub and recently got added  https://expensify.slack.com/archives/C01GTK53T8Q/p1654860503611229

@dhairyasenjaliya Looking at the issue you reported in Slack, it is not the same as this one, but I will ping our QA team to check if they can reproduce reliably and if so, we would create a new issue for it 🙌

@rushatgabhane yeah I think this issue is also different than I thought :)

@dhairyasenjaliya Looking at the issue you reported in Slack, it is not the same as this one, but I will ping our QA team to check if they can reproduce reliably and if so, we would create a new issue for it 🙌

Thank you for the follow-up indeed it's another issue that just got posted in git :)

Correct, the QA team log it for us. Thanks for keeping na eye on this 🙌

You got it @rushatgabhane! Thanks for the input!

Platform:
Where is this issue occurring?
iOS
Android

@mvtglobally this issue mentions that issue is reproducible on Android too. Could you please retest and confirm if this issue occurs on Android too. Thanks!

Price doubled

@mvtglobally this issue mentions that issue is reproducible on Android. Could you please retest and confirm if this issue occurs on Android too. Thanks!

@mvtglobally please. In case you missed this.

Bump @mvtglobally

time to double!

Doubled twice since I somehow missed the prior double

Doubling again

Good thinking @rushatgabhane !

Proposal
The reason why the input is not clearing is because of busy JS Thread and asynchronous nature of React Native.
After user does a submit a lot of JS state updates which creates reconciliation work and locks the JS Thread.
In the meantime, user updates the input on the main thread,since we are interacting with a native text input view, but the problem is that it can not notify JS thread and thus ignores all previous actions done to the field (in this case clear)
setTextAndSelection is what really gets called on clear
Notice the condition on eventLag

Also look at what gets sent over the bridge in the video below
https://we.tl/t-CyPTYk88SP
The solution would be to minimize number of renders(state updates) with the restructure of the report screen(in part or in full), memoization, lazyness etc.
There are multiple strategies for solving perf problems on JS side.

Hi @rushatgabhane
Generally speaking, yes. Fabric allows synchronous updates to native views.
Now, when or how it will be applied to inputs is unknown and adoption of the new architecture is big topic, especially in projects where there is a lot of external dependencies which are not yet updated.
Anyhow, this issue is a just a consequence of a greater problem which is slow ui updates. This will inevitably(with fabric or not) manifest in other ways on slower devices even if clear worked fine.
I would recommend being careful where the state lives and how often the views are updated.

Solving the message got sent but it also stayed in the compose box mystery
Prequel - Analysis of the issue
Please see comment above for initial analysis as to why exactly input is not cleared.
https://github.com/Expensify/App/issues/10148#issuecomment-1248101508
Solution to the issue above is twofold.
Goals:

Make submit faster UI wise
Prioritize clear operation since we have large list of messages and deep report hierarchy so JS Thread can't be 100 percent responsive.

==========================================
1. Make submit faster
We have 2 main complex data structures on report screen.
Report and ReportActions.
Complex data structures are not good for rendering performance since they do not maintain stable reference on update, especially if they contain a lot of nested or unrelated data that are read from different parts of the UI and used for multiple concerns.
React does (OPT-IN) only shallow comparisons during reconciliation and can not figure out if something changed in complex objects and what exactly changed. Besides, shouldComponentUpdate() is hard to maintain and runs on every update, so i avoid adding new shouldComponentUpdate()s for my solution.
1.1 Optimizing on Report  is a lot easier since its not a collection
The problem with Report model is that it covers many concerns and different UI Components are using the same subscription. This is bad since we may update a field in report which needs to render just one part of UI, but it actually renders all components that use same model subscription even if particular state change is not related to them. We are not able to fine-grain subscriptions by default.
Onyx currently does not support fine-grained subscriptions to the particular keys on complex objects. Fine grained subscriptions are really useful and are usually mixed with a selector concept(https://redux.js.org/usage/deriving-data-selectors) which can create derived state -> only updated when specific store state updates which means that computations inside selectors are also memoized.
In order to remediate this, its a lot better to pass exactly the fields UI Component needs for rendering than to pass whole objects as props. PureComponent is compatible with primitive data and this should be used extensively when the need arise.
To avoid spreading all props to components on each level of the tree, we can subscribe to report in a nested components while components higher in the tree, which are using specific Report fields, can benefit from PureComponent
1 .2 Optimizing on report actions is currently hard
Since Onyx does not have stable collection keys on optimistic update, shouldComponentUpdate on reportActions does not work at all for the purposes of report screen submit, even if it is checked for deep equality so everything will re-render twice. Once for optimistc update, and the second time after server returns back 200.
What we can do with reportActions is to pull the state down in the hierarchy as much as we can to avoid whole screen re-rendering
This will create more subscriptions on reportActions and UI will be rendered in separate commits but that’s the trade-off we need to make in deep trees - especially if components are monolithic and are not using compound pattern to share state - https://www.smashingmagazine.com/2021/08/compound-components-react/
Onyx should be able to optimise this in the future by sharing the subscription.
1.3  State updates high in component hierarchy when reports change
MainDrawerNavigator - dangerous - very high in the hierarchy -> calculating initial report -> should be memoized
SidebarLinks -> updated in background even if drawer is not open -> update only if drawer is open
2. Prioritize clear
Use Timers API provided by React Native(https://reactnative.dev/docs/timers)
Specifically - setImmediate and setTimeout(callback) for this.
setImmediate will prioritize clear update
setImmediate is executed at the end of the current JavaScript execution block, right before sending the batched response back to native.
ALERT
While setTimeout is a hack most of the time, especially when you use magic numbers for timeout when you want to synchronise state. In this case it is used without timeout constant, which serves the purpose to deprioritise submit mutation by sending it at the end of event loop queue.
Results
By implementing proposals from above we come to a lot better results than current state.

Clear is done immediately
Report screen is generally more responsive (even in debug mode)- its a lot faster for submit button to become enabled or disabled(even if you type fast) plus less time for new item to be visible in the list.

Test case
Report with around 50 messages already existing
See the videos below for comparison
Old:

279 commits done
https://user-images.githubusercontent.com/11381171/191130926-698df592-15e9-4a0e-bf85-cbc8fe0a8d25.mov
New:

44 commits done plus each commit is faster than commits above
https://user-images.githubusercontent.com/11381171/191130950-96bc504f-54d1-4397-9e0d-be01df66b2ba.mov
Would be even less with reportActions stable keys
Further optimizations to performance need onyx support for the issues mentioned above and possibly larger reorganisation of the code to support those features which can be done incrementally.

@vladamx Super nice breakdown!! It seems like there is a ton of great stuff here, but I'm curious which of these solutions were applied to get the behavior in the video you've shared? And not sure what the proposed course of action would be.
I think we will have a tough time implementing all of these ideas at once (in case this is your official proposal to the TextInput issue - which seems like it is not). It's also a little unclear how we should prioritize them. I think a good next step would be to break these down into separate proposals and we can pitch them as separate improvements.
Out of the things you've explored I'm curious to know:
Which is the solution for the TextInput issue we are discussing here?
Which of the other points related to performance do you think are the highest value to work on and prioritize?
To briefly touch on the things you've mentioned...

Complex data structures are not good for rendering performance since they do not maintain stable reference on update, especially if they contain a lot of nested or unrelated data that are read from different parts of the UI and used for multiple concerns.

I think maybe you are just setting the stage here for the selector idea. Or were you more generally suggesting that we change the data structures we use? I think if it's the latter then we'd probably only want to do that after considering alternatives (since it would be a bit painful to break the complex state into more isolated / simpler objects).

Fine grained subscriptions are really useful and are usually mixed with a selector concept(https://redux.js.org/usage/deriving-data-selectors) which can create derived state -> only updated when specific store state updates which means that computations inside selectors are also memoized.

It's not entirely clear to me if you are proposing to implement some kind of selector pattern in Onyx. Or if you are using this example to illustrate that we should use more specific props + PureComponent or some other solution. This sounds like it would be a good thing to generally propose as a "performance" improvement, but as an entirely separate issue to the current issue.

To avoid spreading all props to components on each level of the tree, we can subscribe to report in a nested components while components higher in the tree, which are using specific Report fields, can benefit from PureComponent

This is a really great point. I think the basic concept can be boiled down to something like - the higher up you are in view hierarchy the more selective you want to be about allowing state updates. And since we are subscribing to some pretty large objects with many fields they are naturally updated more often. I like this train of thought but a clear plan of action or path forward isn't coming to mind for this one. Any ideas on how to move forward on this? Maybe we can do a separate audit + new issues to fix areas that need optimizing + benchmark them?

everything will re-render twice. Once for optimistc update, and the second time after server returns back 200.

If I'm understanding you correctly I think the reportActions issue could be resolved by some future changes to use the reportActionID as a key. The report action object should then stay the same from optimistic update to server response.

What we can do with reportActions is to pull the state down in the hierarchy as much as we can to avoid whole screen re-rendering This will create more subscriptions on reportActions and UI will be rendered in separate commits but that’s the trade-off we need to make in deep trees

This makes sense at a high level, but I didn't quite get the complete proposal. Is there a specific subscription in a specific component that we are moving someplace else? Similar to the issue where we utilize more specific props higher up the view hierarchy this one feels hard to put into practice. But also maybe is it's own separate issue.

setImmediate will prioritize clear update setImmediate is executed at the end of the current JavaScript execution block, right before sending the batched response back to native.
serves the purpose to deprioritise submit mutation by sending it at the end of event loop queue

I think what I'm getting from this is that whatever we are doing in "submit" interferes with the "clear" because it locks up the JS thread and the native text input code ends up canceling the event entirely? And that's why moving the "submit" to happen after the clear with a timer improves things? This still feels a bit hacky to me. Why does the native text input even have this logic to cancel the request to clear? That seems pretty undesirable (but I'm sure I just do not understand why the native code works this way). If we were theoretically doing less "work" after the clear and not blocking things would the clear() be allowed to happen? And if so, maybe messing with timers is not needed?
I agree with @rushatgabhane and would be curious to wait and see if problem still exists in the fabric version of react-native.
Though, I would love to work on fleshing out some more concrete proposals to help improve performance in general across the app - and would maybe even prioritize it higher than the issue at hand. Thanks very much for the detailed analysis - hopefully we can get working on some of these issues together!

Hi Marc,
Actually, this is my official proposal for TextInput issue. posted as a job on Upwork.
As a side-effect you also get better performance on Report Screen in general.

Which is the solution for the TextInput issue we are discussing here?

The solution is UI render performance optimization guided by some of the reasoning in the text above(since that is the heart of the problem with text not clearing) combined with Timers for prioritization of work in order to make TextInput clear behavior consistent.

I think maybe you are just setting the stage here for the selector idea. Or were you more generally suggesting that we change the data structures we use? I think if it's the latter then we'd probably only want to do that after considering alternatives (since it would be a bit painful to break the complex state into more isolated / simpler objects).

Yes, i am setting the stage for the selector idea and proposing it to be implemented in Onyx for a future benefit. I am not suggesting change of data structures since that would be cumbersome at this point.

If I'm understanding you correctly I think the reportActions issue could be resolved by some future changes to use the reportActionID as a key. The report action object should then stay the same from optimistic update to server response.

Right!

I think what I'm getting from this is that whatever we are doing in "submit" interferes with the "clear" because it locks up the JS thread and the native text input code ends up canceling the event entirely? And that's why moving the "submit" to happen after the clear with a timer improves things? This still feels a bit hacky to me. Why does the native text input even have this logic to cancel the request to clear? That seems pretty undesirable (but I'm sure I just do not understand why the native code works this way). If we were theoretically doing less "work" after the clear and not blocking things would the clear() be allowed to happen? And if so, maybe messing with timers is not needed?

The native code works this way because it is not good to lose the user's input you haven't had to time to validate at first place. This would happen if they allow it.
Timers API is mechanism to priroritize work and offload expensive work in react native. Hack is when you use this API to control time, but not when you use it to control the work, if i am making myself clear.
As for the course of action, i can just submit the PR(code speaks the best :)) since changes are relatively straightforward and are guided by some of the principles above.
If you find it valuable and applicable then great.

not overdue

Timers API is mechanism to priroritize work and offload expensive work in react native. Hack is when you use this API to control(bend) time, but not when you use it to control(bend) the work, if i am making myself clear.

I am not sure if we agree on this point yet. But, I'm curious to see how bad this TextInput issue is after the performance improvements are implemented. Is there a point where we can avoid using a setTimeout() or setImmediate()?
I'm going to discuss this internally and get back to you on whether we'd move forward on the Timer based solution. I looked at the code for this and it is a bit confusing (especially the setTextInputShouldClear() method logic).
I do think the other optimizations are worth pursuing and wouldn't mind working to create separate issues for them.

it is not good to lose the user's input you haven't had time to validate at first place

I'm not sure what is meant by "validate" in this case. But at a high level, if we are storing the value of the text input and clearing the input before we submit the comment in the JS then this should work correctly?

I'm not sure what is meant by "validate" in this case. But at a high level, if we are storing the value of the text input and clearing the input before we submit the comment in the JS then this should work correctly?

"validate" in this case means that you ordered a clear from JS for "marc" but not for "marco" since new "o" input came in before clear was able to complete.  Since it would not be fair or good to lose users's input "marco" rn bails out.

"validate" in this case means that you ordered a clear from JS for "marc" but not for "marco" since new "o" input came in before clear was able to complete. Since it would not be fair or good to lose users's input "marco" rn bails out.

Ok, I think I've got the point now, thanks! Still seems not very ideal. We are pretty strict on not allowing these kinds of workarounds so I will get some more thoughts and provide an answer soon.

Did some testing and I'm still able to reproduce the bug on that branch provided on iOS. I think most likely we want a different solution for the TextInput bug. I would be curious to see some of the proposals related to performance in a separate issue with some benchmarks.

not voerdue

any updates cc @rushatgabhane ?

I'm able to (sometimes) reproduce this on Desktop, should the OP updated to include Desktop? I think you might have to send a chat to someone you haven't for a little while (or possibly to someone who messaged you last).
https://user-images.githubusercontent.com/22508304/193172232-ce98a11a-b6ae-426a-a210-62060dac3d7e.mp4

I think this might be related to this issue https://github.com/Expensify/App/issues/11526

I report this issue before https://github.com/Expensify/App/issues/7473

@rushatgabhane any updates here?

Moving to Weekly

I can't reproduce this bug anymore in Android. @dylanexpensify can you check on iOS? I think if neither of both can, we are good to close it.

Just moving to daily so we don't forget to try to reproduce. @rushatgabhane Can you see if you can reproduce it?

I think this might have been improved but not fixed.  I'm able to reproduce on iOS app, v1.2.14-0 if i paste a large block of text (from here), hit send, then immediately start typing right after.
@youssef-lr , you worked on https://github.com/Expensify/App/issues/11526   do you think it's related? If so, thoughts on why it's still occurring with large blocks of text?
https://user-images.githubusercontent.com/22508304/195633527-862fdbce-0f00-4318-a868-3184f29c38ab.MP4

In android I can only reproduce it if I type really really fast 😆 It stays for less than a second, but yeah, maybe we can improve it somehow.

@youssef-lr any thoughts on this?

@mallenexpensify @dylanexpensify sorry guys I missed this! I think this is different from the issue I worked on and is exclusive to iOS and Android due to React Native as explained here, it seems like it's a little bit tricky to fix without resorting to using timers. I will look into it further this week.

Thanks @youssef-lr! Thoughts on moving this to weekly @AndrewGable @rushatgabhane?

If it's a reproducible bug I think we should fix it.  Partly cuz... I have a feeling that it'll become more prevalent as we have more and more chats in our profile or the app.  It appears to be reproducible in both iOS and Android too.  If we think something else might improve or fix this, I could see putting it on hold though.  ¯_(ツ)_/¯

Personally, I'd be ok with closing. If it does pop up in the instance you're thinking @mallenexpensify then we can open a new issue. Thoughts? cc @pecanoro

@dylanexpensify I suggest bringing it up in Slack to see if we should keep it, close it or move it to a new issue.

brought up here!

@rushatgabhane done!

Have we ruled out that this could possibly fixed with "Deprecate sequence number"?

Have we ruled out that this could possibly fixed with "Deprecate sequence number"?
cc @roryabraham and @tylerkaraszewski since you're working on.
Internal issue https://github.com/Expensify/Expensify/issues/227234

Are you still accepting proposals for this?

@Julesssss suggests in #expensify-open-source

I’d say close. While it’s important to improve performance, it’s going to be difficult to attribute a single fix to any perf improvement here. We’ll likely see this issue disappear as we implement bigger perf improvements elsewhere

@AndrewGable you agree?!??!

I am in favor of closing 👍

K, gonna close for now.  It doesn't mean it can't ever be reopened, we're just deciding to focus on hundreds of other bugs

Haven't seen this issue happening recently

Heads up, this came up again above. We probably have more time to focus on this one since we have a much better handle on WAQ.

I'm going to reopen it so we can have a conversation about it again.

I did a side-by-side comparison between Whatsapp and EChat and I can see it does still happen. If there's anything I can do to help push this along I'll be happy to!
https://user-images.githubusercontent.com/46995600/235756441-b01a0a81-b4ce-49a9-b6b0-d6d8c4ceb4d0.MP4

bump @muttmuure!

Sorry, I won't be able to get to this today. I can push it forward when I'm back online next week.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Messages don't clear from composer straight away after sending them
What is the root cause of that problem?
This happens because we have a 1 second debounce on saving drafts
What changes do you think we should make in order to solve the problem?
We should either reduce the debounce time or remove the debounce completely.

Proposal
Please re-state the problem that we are trying to solve in this issue.
When the "Send" button is pressed, and the text input is quickly edited, the text input is not effectively cleared.
What is the root cause of that problem?
In React Native, native components' interactions are processed in a separate thread from the JavaScript code. When some state can be manipulated both interactively by the user (in the UI thread) and via JavaScript (from the JavaScript thread), and the JavaScript blocks, pathologies can occur.
In this case, we indeed block the JavaScript thread. I localized two Onyx operations that contribute to the thread being blocked:

The optimistic data update triggered indirectly by Report.addComment
The merge triggered by Report.setReportWithDraft

What changes do you think we should make in order to solve the problem?
We can debounce both operations.

Report.addComment can be debounced in the ReportScreen component
Report.setReportWithDraft can be debounced in the ReportActionCompose component

Debouncing Onyx-triggering actions is already done in this file. I propose to expand it to more operations. Quoting the appropriate comment:

Save our report comment in Onyx. We debounce this method in the constructor so that it's not called too often
to update Onyx and re-render this component.

Any updates @priyeshshah11 ?

I updated the proposal. I redacted the text to make it easier to understand, I added a deeper explanation of the underlying problem, and I provided a link to the experimentation tool I used and a video of the mentioned tool.

@priyeshshah11 Thanks for the feedback! Were you testing on the debug build or the release build? What device were you using?

@priyeshshah11 Thanks for the feedback! Were you testing on the debug build or the release build? What device were you using?

debug build on iOS simulator iphone 14 pro - iOS 16.4

@rushatgabhane & @AndrewGable Updated my proposal here

@priyeshshah11 On the release build, I could not reproduce the issue after applying my changes.
By the way, I'm not really sure about what you mean when you say that "we don't clear the composer straight away". We do, that operation is just not immediately reflected in the state of the native component, as React Native doesn't work this way.

@AndrewGable should this stay external? Or do we want to move it internally?

@dylanexpensify What's the reason for this suggestion? What's wrong with the process so far?

Leaving reviews up to @rushatgabhane !

@dylanexpensify What's the reason for this suggestion? What's wrong with the process so far?

@cubuspl42 it's because this issue is several weeks old, and we will often move one internal if not solved within 4 weeks!
@rushatgabhane let's prioritize this one for review since it's several weeks old, please 🙇♂

@rushatgabhane

I don't like that because the comment isn't immediately added to the report screen which makes the app 'feel' slow.

Have you tested the release build? Also, did the chat "feel" fast before applying my commit? I added debounces in the range of 10 - 100 ms. Such debounces can't be directly responsible for the multi-second delay you're observing.
Before applying my commit, I could easily reproduce this bug on the release build. After applying this commit, it was virtually impossible.

We should reset the composer as soon as the send button is pressed

@priyeshshah11 could you please help me understand this? If I'm not wrong, we're already doing that. setState(comment("")) is async so I don't know what else we can do to improve it.

Yes we do clear it ASAP but we do a lot of things along with it such as onyx updates, etc which blocks the JS thread & by the time our empty state passes down to the native textInput, it gets overwritten by it's onChange which gets triggered with the old comment + new characters thus it looks like the composer was never cleared.

@priyeshshah11 could you please attach a diff / branch with all your suggested code changes?

@rushatgabhane I am suggesting something simple like this
submitForm(e) {
        if (e) {
            e.preventDefault();
        }

        this.setState({value: ''});

        setTimeout(() => {
            // Since we're submitting the form here which should clear the composer
            // We don't really care about saving the draft the user was typing
            // We need to make sure an empty draft gets saved instead
            this.debouncedSaveReportComment.cancel();

            const comment = this.prepareCommentAndResetComposer();
            if (!comment) {
                return;
            }

            this.props.onSubmit(comment);
        });
    }

https://github.com/Expensify/App/assets/38547776/463d8bdb-eeca-4ead-b6ce-13d9799abfdd

@ahmdshrif You address the problem as directly as the other proposals. You would address it more directly if you made the performance problem go away. Instead, you just totally left it out of your root cause analysis and didn't consider it your solution.
It doesn't work like that. It still exists, you just picked different trade-offs.
When the user types / sends messages fast, depending on muscle memory (which is the real-life case when our bug actually manifests), your solution is prone to dropping inputted letters. I'm not speaking hypothetically, I've built it and seen it.
I'm leaving apart the fact that your diff has typos in symbol names (was it tested at all?).

@rushatgabhane let's get an update here please! 🙇♂

I quickly created a 1:1 comparison of the same conversation performed on a branch with my proposal applied and on a branch using the shouldWaitUntilReset proposal.
My proposal (debouncing Onyx operations):
https://github.com/Expensify/App/assets/2590174/1a192a14-b3c2-44cd-b51d-f0abad5f2446

shouldWaitUntilReset proposal:
https://github.com/Expensify/App/assets/2590174/1bd2a3a9-d378-4aae-8e36-2b1bdc1db4f3

@ahmdshrif

Be fair when you test you slow down in your test and only type after messages get sent in your solution and speed up in my solution and keep typing when you press the send button😄

That's exactly the idea that I typed after pressing "Send"; the whole issue is about typing after pressing "Send". In my proposal, the message just leaves the composer earlier (but takes slightly longer to appear in the chat log, summing up to essentially the same total delay).
I just slowed down the videos and in both cases, the first letters of the messages are typed before the previous message appears in the chat log.

For your info, we have C+ to test proposals and recommend the best, so don't waste your time testing other's proposals, and that's the reason why I am not testing your proposal

Thanks for the info.  You included other proposals in your own one, as "alternatives you don't recommend". I'm responding to that.
I know the process and I know its limitations. No need to worry about my time.

@rushatgabhane mind giving some next steps here.
Also reminder to everyone to ensure we're writing and acting with humility and respect. ❤

I'm heading OOO tomorrow for about a week so adding another BZ member while I'm away to help keep the train moving! 🚂 Thanks @slafortune!!

Promise, no hot potato here!  I'm heading OOO until June 6th.  @MitchExpensify  - You're the lucky buddy on this for @dylanexpensify

Bumping @dylanexpensify's last request:
@rushatgabhane mind giving some next steps here
Thanks!

Proposal
Please re-state the problem that we are trying to solve in this issue.
Submitting message should clear the input instantly.
What is the root cause of that problem?
Submitting new message starts a set of operations including heavy Onyx state changes and FlatList component re-render, which results in flooding JS thread. Resetting input value happens between component's render cycles while thread is busy, which is the root cause of the problem.
What changes do you think we should make in order to solve the problem?

We can discuss this unorthodox solution. Create native components for <Input /> and <Button /> (objc, java). Replace composer with newly created native component. Pressing submit button will reset the input (on native land) before dispatching onPress event.
Implemented a very basic PoC, it seems to be very performant with no delay as we avoid any double trips between threads.  However this should be discussed internally as it's the new of fixing things. Taking into account that this would require more work for both Paper and Fabric implementations, I suggest we double the bounty for the described approach if selected.
What alternative solutions did you explore? (Optional)

@rushatgabhane Just to be on the same page, on which build are you running your tests? Debug, or release?
These solutions (and I mean both the timeout-based and the shouldWaitUntilReset one) have highly timing-sensitive side effects. I don't think any conclusions can be drawn from testing them on the debug build.

@rushatgabhane Can you link the slack discussion please. @azimgd solution seems unorthodox as he mentioned but also it must be quite performant

Bump:

@rushatgabhane Can you link the slack discussion please. @azimgd solution seems unorthodox as he mentioned but also it must be quite performant

I updated my proposal with the results of my systematic tests, which include precise measurements. I hope that it will help clarify some misunderstandings.

Robert and Tomek from SWM will help @rushatgabhane to get to the bottom of this issue. They will go through the existing proposals and research to get more context and see what are our options.

Proposal
Please re-state the problem that we are trying to solve in this issue.
The message stays in the compose box even after the message is sent.
What is the root cause of that problem?
To me, it looks like the root cause of the problem is the predictive text feature that prevents the normal functioning of our composer.
To confirm this, we can just disable the predictive text feature on native devices by configuring the mobile settings and the problem disappears.
What changes do you think we should make in order to solve the problem?
To solve the problem, the approach was taken to collect the keyboard inputs provided by the user and, gracefully, resetting the composer with the remaining message comments.
A little more detailed explanation for the approach is as follows:
Whenever submit of a message is initiated, we will enter into the state of managing predictive text using shouldManagePredictiveText. Once we enter into this state, we will track state changes to value and use debouncing to wait until the user has stopped keying inputs via keyboard. Once componentDidUpdate becomes silent for some time (I used 200 ms), we will decide whether there is a need to update the composer message and update the composer message accordingly so that the user does not lose any message contents. Then, we will reset the shouldManagePredictiveText state to come out of the predictive text management.
Here is the code reference for my test result video.
a) Initializations:
  this.debouncedPredictiveTextHandler = _.debounce(this.debouncedPredictiveTextHandler.bind(this), 200, false);
  this.prevSubmittedComment = '';

shouldManagePredictiveText: false,

b) Whenever, there is change in value for which we need to manage predictive text, launch the debounced predictive handler. This will ensure that the pending keyboard inputs are collected. Further, we will wait for 200 ms of silence in updates to states/props before we decide to halt the predictive text handler and update the TextInput with the new value.
Just above the following line in componentDidUpdate(prevProps, prevStates)
const shouldSyncComment = prevProps.comment !== this.props.comment && this.state.value !== this.props.comment;
add the following code:
       if(prevStates.value !== this.state.value || this.state.value !== this.comment)
       {
           if(this.state.shouldManagePredictiveText)
           {
               console.log("PREDICTIVE_HANDLER, prevStates.value["+prevStates.value+"],this.state.value["+this.state.value+"],prevSubmittedComment["+this.prevSubmittedComment+"],this.comment["+this.comment+"],");
               this.debouncedPredictiveTextHandler(this.state.value);
           }
           else
           {
               console.log("UPDATECOMMENT (EXEC-1), this.comment["+this.comment+"],this.state.value["+this.state.value+"],propscomment,PREV["+prevProps.comment+"],CUR["+ this.props.comment+"]");
               this.updateComment(this.state.value);
           }
           return;
       }

c) Add a state  shouldManagePredictiveText to determine start and stop of the management of predictive text.
    setShouldManagePredictiveText(shouldManage) {
        console.log("setShouldManagePredictiveText["+shouldManage+"]");
        if(!shouldManage)
            this.prevSubmittedComment = "";
        this.setState({shouldManagePredictiveText: shouldManage});
    }

d) Here is the debounced function.
    debouncedPredictiveTextHandler(newComment) {
        const isPredictiveText = newComment.startsWith(this.prevSubmittedComment);
        if(this.state.shouldManagePredictiveText)
        {
            if(isPredictiveText || newComment.length === 0)
            {
                const newCmnt = newComment.substr(this.prevSubmittedComment.length);
                this.updateComment(newCmnt.length!==0 ? newCmnt : newComment);
            }
            else
            {
                this.setShouldManagePredictiveText(false);
            }
        }
    }

e) In prepareCommentAndResetComposer() , replace this.updateComment(''); with the following
        this.prevSubmittedComment = trimmedComment;
        this.setShouldManagePredictiveText(true);
        this.setState({value: ''});

f) In render(), use setState for onChangeText
    <Composer
	onChangeText={(comment) => this.setState({value: comment})}

Test Video:
https://github.com/Expensify/App/assets/3004659/c5468d04-8298-4814-b382-cbcaa7f72726
What alternative solutions did you explore? (Optional)
The approach of preventing Predictive Text functionality for the Composer momentarily at the time of sending message was thought of. But, I could not make it work with TextInput props like AutoCorrect and AutoComplete. Also, Another way was to switch keyboardType to something like visible-password and bring it back once submit is completed. But, this will clash with multiline textinput that we use. So, I did not proceed with this. Another approach was to use RNTextInputReset.resetKeyboardInput. But, I do not see value in using this as it is only for Android. Not sure, how we manage with all other platforms?

Proposal
Please re-state the problem that we are trying to solve in this issue.
The message got sent but it also stayed in the compose box.
What is the root cause of that problem?
I. This is a problem from React Native and their way of communicating with Native Apps.
Firstly, I have created a new React Native project and added 1 simple component with 1 TextInput and 1 Button. You may see the same problem happening here:
https://github.com/Expensify/App/assets/56603839/ad21d08c-6a39-4e19-943d-7837bdf19152
And here you may see the component inside the public repo I've created.
II. But, why?
React Native relies on a bridge to communicate between the JavaScript code and the native UI components on the device. This bridge introduces some overhead, especially when it comes to handling frequent state updates. In my example we have only 1 setState so you have to be very fast to reproduce this bug, I don't think a user in practice would send messages so fast. In Expensify, there are a lot of Onyx and state updates right now, and the lag between pressing on Send Button and actually having the input cleared is pretty high as you may see here:
https://github.com/Expensify/App/assets/56603839/7293d79b-e7b8-4c0c-8c61-b56d1a547d6a
If a user writes "H", then presses on Send Button, there's a large time window in which the "H" message still remains in the Native Text component even though the value in state is cleared in Composer component. If user writes "h" in that time window, onChangeText callback will return "Hh" thus setting the new value in state to "Hh" in Composer component.
What changes do you think we should make in order to solve the problem?
Either I. Create an issue regarding the problem happening in a new project on react-native github repo and see the feedback. I've found some similar issues, but not this one.
or II. Implement a workaround. There are some workarounds already, are you open to other ones?
or III. Optimize our component as to reduce the delay such that, in practice, fast typing users won't be bothered by this bug. You may see in the first video that the time window between pressing the Send Button and actually having the input cleared is pretty low. (Optimization ideas: reduce the number of re-renders, use refs instead of state where possible, reduce Onyx processing). Should I try and create a fork with optimizations?
Or, why not, all 3 of the above.

@rushatgabhane

Thanks for creating this reproducible version

A reproducible MCVE has been available for mutliple weeks.

Still, another one definitely won't hurt.

@rushatgabhane

In Fabric, isn't this synchronous? Is this bug still reproducible if you make use of Fabric in the example app you've created?

I've made a new branch called fabric where I've set newArchEnabled=true in gradle.properties. As per this log:

And this is the result:
https://github.com/Expensify/App/assets/56603839/aba2fdd8-dd8e-4aa3-8cb5-e1f2b6ee320a

The bug still happens, it feels that I have to write slightly faster to reproduce the bug, or it may be only placebo.
The text color is buggy. First letter changes from white to black, then text remains black. I did not change anything.

We may also add the above video to the issue in react-native github repo, if one is to be created.

Further updates to my proposal:
Android/Chrome: On this platform, the predictive text seems to be the cause. When the predictive text feature of Samsung keyboard was disabled, the problem could not be reproduced.
Android/Native: On this platform, the predictive text does not seem to cause the problem. Here, when the predictive text feature is disabled, the problem is easily reproduced.
Since keyboard implementations by device manufacturers intercept text inputs for various reasons (like predictive text), I do not think we can safely rely on the text input value when we submit the text message in chat and also maintain focus within the composer at the same time. Since device manufacturers' implementation is also involved here, it will be difficult to get consistency across all devices. This is where my proposed approach helps as we remain insulated from the problems introduced by device manufacturers.
Please refer a recent issue here as an example of how device manufacturers can mess up implementations. Alternatively, react-native framework can also insulate themselves from problems introduced by device manufacturers but it could involve redesign efforts.
@rushatgabhane, I have updated my proposal and have also shared additional test video for Android/Native platform and test branch details. Would love to have your feedback on proposal.

@cubuspl42  @DanutGavrus @rushatgabhane  I was trying to understand your comments on the root cause and have few queries on this.
As I understand, the react-native framework will use an event-based mechanism to handle all inputs in the composer. If we look from that perspective, in the onMouseDown event, we handle submit and, in the same handler, we have injected the event to reset value to an empty string. Later, when the user keyed into the composer, the framework would have pushed it into the event queue. Now, the event handlers would have executed in the same order (i.e. a) reset composer to an empty string, b) handle key input event in composer) so that sanity prevails in the system.
Now, the only reason I see for a bug to occur would be if a) the framework did not synchronously update the composer with an empty string. In this case, reading the current value from the composer can go wrong at a later point. or b) the framework did not push into the event queue when the user keys into the composer. Again, here, reading the current value from the composer can go wrong. I seriously doubt if the framework owners would make any of these mistakes unless the device manufacturers play havoc here. I haven't look at the react-native code, but the bridge communication between the JavaScript code and the native UI components on the device also should have managed it properly. Where do you see the problem? Or am I missing something here? Please let me know.

The TextInput component allows direct user manipulation and communicates the text's changes via the onChangeText callback. Simultaneously, it accepts a value property that controls the text input, expecting this value to update according to onChangeText changes. If this update doesn't occur, the user-inputted changes are reversed.

If a user writes "H", then presses on Send Button, there's a large time window in which the "H" message still remains in the Native Text component even though the value in state is cleared in Composer component. If user writes "h" in that time window, onChangeText callback will return "Hh" thus setting the new value in state to "Hh" in Composer component.

I also did a small exercise on Android/Chrome platform to print logs to understand the behavior in the current application and am attaching the screenshot for the same. In the screenshot, CUR represents the this.props.comment value. Here, I notice that, first, the this.props.comment gets reset to an empty string and, then, the changeText for the next input arrives. But, then, changeText did not factor in the empty string and the problem got reproduced. With respect to the root cause, I am trying to reconcile the results of my exercise with your above-mentioned comments. Can you please help me on this?

@rojiphil @rushatgabhane
Let's consider A - React component(which holds the state and the actual text content in the Javascript's text input) and B - Native counterpart(in Android for example, this holds the actual text content in the edit text, where EditText is the Android's native text input).
Analyzing the behavior:

Type 'H'. Press on Send Button. JS code starts executing. Checking A's state inside render method shows '' after <5ms, and '' in all next renders, meaning that the state was actually cleared.
1.1 Accessing the private fields of A, the text content is also '' inside it in all next renders.
But, we see 'H' on screen for <1000ms until it eventually clears.
2.1 This means that, for some reason, B's text is not cleared in this time window. Thus, if user types 'h' in this time window, the onChangeText method callback retrieves the text from native and returns 'Hh' to A. JS code starts executing again according to the returned value, resulting the bug.
2.2 for some reason = bridge's overhead.
Bug is reproducible in a new React app, but with a way smaller time window.
3.1 Doing optimizations to our component, reduces the time window to be close to a new React app.
3.2 Enabling Fabric in the new React app does not get rid of the bug.
Bug is not reproducible in a new native(Android or iOS) app.
Bug is not reproducible on Web.

We may conclude that:

It's a bug inside react-native.
It's related to the implementation of the communication between Javascript's TextInput and Native's counterparts.

And we may:

Optimize the app to have a time window as close as possible to a new native app.
Report the issue in react-native repo to get more context.

What are your opinions on this?

Not overdue Melvin

As this idea was already mentioned in this thread multiple times, I decided to actually file a bug report to React Native. Whatever they say, it will be a valuable input to this discussion.

@rushatgabhane @cubuspl42 I think someone from the Expensify team should have chosen who to post the issue in React Native repo based on who was the first to propose that / who did enough research.
I don't see this suggestion in your proposal, or in your other comments.
Besides, you used the title TextInput desynchronizes with its native counterpart in the react-native Issue, where out of ~250 comments on this issue and the old one, I was the only one who used the term counterpart / Native counterpart 7 hours ago when I re-explained the bug.
Also, it seems that you didn't test it with Fabric in order to specify in the issue if it's still reproducible.

@DanutGavrus I don't think that either you or Expensify team has any power over who can post issues to a public GitHub repo, but I respect your differing opinion.

I think @MitchExpensify is looking for help on this one

I think @MitchExpensify is looking for help on this one

Latest internally is @robertKozik is working with Tomasz to see if they can come up with a proposal (Slack thread here)

Woooot! @MitchExpensify I'm back from OOO now, and can take this back!! TY so much for all the help with it!

Wooh! Thanks @dylanexpensify

hey folks, dropping this here as it might be related, it's not yet merged: https://github.com/facebook/react-native/pull/29580

Just to confirm @MitchExpensify have we found someone to help with this from expert contributors??

Yeah, me and @tomekzaw  from Software Mansion are looking into it.

Proposal
Please re-state the problem that we are trying to solve in this issue.
text value on the TextInput component continues to exist sometimes after submission.
What is the root cause of that problem?
User interaction is on UI thread, and we use JS Thread for state management.
The text update event (e.g.: onChangeText) from the UI thread contains the previous text, if the user inputs any text between 1⃣ & 2⃣ in the following diagram.
sequenceDiagram
    actor U as User
    participant UI as UI Thread
    participant JS as JS Thread

    U ->> UI: input text

    UI ->> JS: input update event
    JS ->> JS: update text

    U ->> UI: submit
    UI ->> JS: 1⃣ fire `submitForm()`
    JS ->> JS: do stuff
    rect rgb(255, 100, 100, 0.1)
      opt THIS IS THE root cause of the problem
        U ->> UI: input text
        UI ->> JS: input update event 🔴<br />(previous text and new text)
      end
    end
    JS ->> UI: 2⃣ clear text <br/>(text is cleared here)
    UI ->> UI: clear text
    JS ->> JS: update text 🔴<br />JS is single thread so updates<br />should happen after `clear()`
    rect rgb(255, 100, 100, 0.1)
      JS ->> UI: update UI by 🔴 (update `value` prop) 💥
      UI ->> U: User see previous text and text from 🔴
    end

What changes do you think we should make in order to solve the problem?

As we handle logic and store state in JS Thread, we need to ensure data integrity in JS Thread.
So what we can do is, for example, set a flag like this.isSubmitting = true here
https://github.com/Expensify/App/blob/4fdc8dbec2cbd74190a68ca5008092a2dbbdbc16/src/pages/home/report/ReportActionCompose.js#L868-L872
, and use it to keep input characters while submitting.
This action is not the perfect solution as users can see "previous text + new text" until inputRef.clear() will be called (and fix inputs as it should be), but it's better than the current situation (keeping previous text).
It's also possible to implement another TextInput component or extend the TextInput component from native side (obj-c & java & cpp).
If we can set isSubmitting flag on the native side, we can minimize the time that users see "previous text + new text".
But even with this implementation, there is a slight chance to get "previous text + new text" as it's possible for users to input new text before we set isSubmitting flag on the native side.

Proposal
Updated
The changes I proposed was unclear so I added a bit more text.

Proposal
Please re-state the problem that we are trying to solve in this issue.
The text is not cleared in the Composer.
The internal state of the native iOS Component (RCTUITextView)  or Android Component (ReactEditText) overrides the JavaScript-TextInput state with the onChangeText callback.
What is the root cause of that problem?
The issue is discussed in Parashuram's presentation at ChainReact 2018 (more info in my comment). The root cause of this issue is https://github.com/facebook/react-native/issues/20119.
When the user types Hello, clears the text and adds a new letter d:

The clearing of the text is delayed because the JavaScript thread is busy.
The update of the internal state of the native iOS (RCTUITextView) or Android (ReactEditText) is delayed.
iOS (RCTUITextView) or Android (ReactEditText) sends an update to TextInput.js via onChangeText after the user types a new letter. The iOS/Android TextView internal value was not cleared. The value sent from native to JS is Hellod instead of D.
JavaScript TextInput.js changes the internal value of the TextInput to Hellod.

The issue affects Fabric and Paper renderer. The below tests case are on Fabric, but the same solution should be implemented on Paper renderer.
Reproducing the issue with the  Fabric renderer on React Native (MRE)

Reproducing the issue with the Fabric renderer on React Native (MRE)
When the user types Hello, clears the text and adds a new letter d:

The text is cleared with setText("")
The JavaScript thread is busy (more info in this comment). The re-render and the prop update are delayed, props.value "" is not immediately passed to the native Component iOS UITextView or Android ReactEditText.
The user types a new character d in the native Component (on iOS UITextView, Android TextView). The native UITextView did not yet receive the new text value "" (step 3) via props.value from javascript. The internal state UITextView text value is still “Hello”
UITextView changes the internal state to Hellod and  triggers textDidChange. Native passes Hellod to javascript TextInput
onChangeText
JavaScript saves the value received from Native (Hellod) in its internal state.

CLICK TO OPEN SOURCE CODE

import React, {useState} from 'react';
import {View, StyleSheet, TextInput, Button} from 'react-native';

function wait(ms) {
  var start = Date.now(),
    now = start;
  while (now - start < ms) {
    now = Date.now();
  }
}

const ControlledTextInput = () => {
  const [text, setText] = useState('A long string');
  let textInputRef = React.useRef();

  const clear = () => {
    console.log('clearing text');
    // if (textInputRef && textInputRef.current) {
    //  textInputRef.current.setNativeProps({text: ''});
    // }
    setText('');
  };

  const onChangeTextSync = newText => {
    console.log('onChangeTextSync called with newText: ' + newText);
    setText(newText);
  };

  wait(1000);

  return (
    <View style={styles.container}>
      <TextInput
        ref={textInputRef}
        style={styles.textInput}
        value={text}
        onChangeText={newText => {
            console.log('onChangeText called with newText: ' + newText);
            setText(newText);
        }}
        // unstable_onChangeTextSync={onChangeTextSync}
      />
      <Button title="Clear text input" onPress={clear} />
    </View>
  );
};

const styles = StyleSheet.create({
  container: {
    marginTop: 300,
  },
  textInput: {
    margin: 10,
    height: 50,
    borderWidth: 1,
  },
});

function App(): JSX.Element {
  return <ControlledTextInput />;
}

export default App;

What changes do you think we should make in order to solve the problem?

Clear the value of the native UITextView/ReactEditText using setNativeProps({text: ""}).
Implement an adapted version of unstable_onChangeTextSync on react-native Paper (see my comment from 2020 and commit https://github.com/facebook/react-native/commit/8c6a98400ee861fec757dd7135b217783852be70)
Use unstable_onChangeTextSync instead of onChangeText

Fixing the issue with the  Fabric renderer on React Native (MRE)

Fixing the issue with the Fabric renderer on React Native (MRE)
When the user types Hello, clears the text and adds a new letter D:

JavaScript updates the iOS native RCTUITextView internal state with textInputRef.setNativeProps({text: ""}).
JavaScript updates internal state with setText("")
The JavaScript thread is busy (more info in this comment). The re-render and the prop update are delayed, props.value "" is not immediately passed to the native Component iOS UITextView or Android ReactEditText.
The user types a new character D in the native Component (on iOS UITextView, on Android TextView). The native UITextView text value was cleared with setNativeProps in step 1. The internal state UITextView text value is “”.
UITextView changes the internal state to "D" and triggers textDidChange. Native passes "D" to javascript TextInput
onChangeTextSync. onChangeTextSync calls are queued synchronously with EventPriority::SynchronousBatched (more info in https://github.com/facebook/react-native/commit/8c6a98400ee861fec757dd7135b217783852be70)
JavaScript saves the value received from Native (D) in its internal state.

CLICK TO OPEN SOURCE CODE

import React, {useState} from 'react';
import {View, StyleSheet, TextInput, Button} from 'react-native';

function wait(ms) {
  var start = Date.now(),
    now = start;
  while (now - start < ms) {
    now = Date.now();
  }
}

const ControlledTextInput = () => {
  const [text, setText] = useState('A long string');
  let textInputRef = React.useRef();

  const clear = () => {
    console.log('clearing text');
    if (textInputRef && textInputRef.current) {
      textInputRef.current.setNativeProps({text: ''});
    }
    setText('');
  };

  const onChangeTextSync = newText => {
    console.log('onChangeTextSync called with newText: ' + newText);
    setText(newText);
  };

  wait(1000);

  return (
    <View style={styles.container}>
      <TextInput
        ref={textInputRef}
        style={styles.textInput}
        value={text}
        // onChangeText={newText => {
        //      console.log('onChangeText called with newText: ' + newText);
        //    setText(newText);
        //  }}
        unstable_onChangeTextSync={onChangeTextSync}
      />
      <Button title="Clear text input" onPress={clear} />
    </View>
  );
};

const styles = StyleSheet.create({
  container: {
    marginTop: 300,
  },
  textInput: {
    margin: 10,
    height: 50,
    borderWidth: 1,
  },
});

function App(): JSX.Element {
  return <ControlledTextInput />;
}

export default App;

@rushatgabhane can we get these reviewed asap today? Thank you!! 🙇♂

Ah shoot, no you're right, no need to review, thank you!

Proposal
Please re-state the problem that we are trying to solve in this issue.

User has entered text "Hello"
User hits the send button
User starts typing "world" (as a separate message)
The message box contains "Helloworld" but the expected value is only "world" ❌

What is the root cause of that problem?
Here's what happens when sending a message:

When user hits the send button, the application receives an event on the UI thread.
The native part of React Native emits an event which goes through the bridge and schedules a call on the JS thread.
The callback runs setText("") which causes a React render on the JS thread.
The JS part of React Native sends UIManager instructions via the bridge.
The value of TextInput is cleared on the UI thread.

Because of these UI → JS → UI jumps, there's a time window when the user can still edit the native text input before it's cleared as a result of setText("") call. During that time:

Tapping on the keyboard will actually append to the previous message since it still contains the old value. As a side-effect, editing the value will increase view.nativeEventCount counter.
Calling setText("") will have no effect on the native text input control since mostRecentEventCount will be outdated, so the update will be skipped (see here).

What changes do you think we should make in order to solve the problem?
Here's our goals in terms of the expected behavior and user experience:

The compose box is cleared as soon as the user hits the send button

The message does not stay in the box for any amount of time

The user can start writing next message as soon as the user hits the send button

The keyboard always stays open
We don't block the input for the time of sending the message
We don't clear the text input after some time

The approach that we recommend is to clear the text input directly on the UI thread (using Reanimated worklets) immediately after the send button is pressed (detected using React Native Gesture Handler) as well as to take advantage of this code so that setText("") call only affects the React state but not the native state of the text input.

Wrap the send button inside a <GestureDetector> from react-native-gesture-handler (already used in the app) so that we can react to the event without any delay while still on the UI thread:

import { Gesture, GestureDetector } from 'react-native-gesture-handler';

<GestureDetector gesture={tap}>
  {/* send button */}
</GestureDetector>

Convert the underlying TextInput into an Animated component so that we can take an animated ref and pass it to a worklet:

import Animated, { useAnimatedRef } from 'react-native-reanimated';

const AnimatedTextInput = Animated.createAnimatedComponent(TextInput);

const animatedRef = useAnimatedRef();

<AnimatedTextInput {/* props */} ref={animatedRef} />

Call updateProps function from Reanimated when the send button is pressed:

const tap = Gesture.Tap().onEnd(() => {
  'worklet';
  const viewTag = animatedRef();
  const viewName = 'RCTSinglelineTextInputView';
  const updates = {text: ''};
  _updatePropsPaper(viewTag, viewName, updates); // clears native text input on the UI thread
  runOnJS(send)(); // calls `setText('');` to update React state
});

(Optionally) Submit a PR to Reanimated that implements updateProps method directly on animated refs (easy)

 const tap = Gesture.Tap().onEnd(() => {
  'worklet';
-  const viewTag = animatedRef();
-  const viewName = 'RCTSinglelineTextInputView';
-  const updates = {text: ''};
+  animatedRef.current.updateProps({ text: '' }); // clears native text input on the UI thread
   runOnJS(send)(); // calls `setText('');` to update React state
 });

Wow, thanks for such a detailed explanation and root cause analysis. I think the solution looks very interesting and its great we could us more of the Reanimated magic.
One question: Do you think there could be any side-effect or problems with this solution with the edit functionality? We allow users to edit multiple messages at one go where you can open multiple edit textInputs as you can see here

Any concerns with this solution?

@mountiny At this point I can't see any downsides of this solution when it comes to the edit functionality. What's more, in this case, we could even disable the text input as soon as the user hits the save button (from a worklet on the UI thread) so that he can't modify the edited message afterwards (before React changes text input into a regular text). Alternatively, we could move focus to the next text input (once https://github.com/software-mansion/react-native-reanimated/pull/4586 lands).

I've checked on my rough implementation of the proposal, editting functionality works as before:
https://github.com/Expensify/App/assets/61146594/d522d4cf-f3ba-4913-98de-7516f765f302

Thanks for confirming

@rushatgabhane Thanks for these perfectly valid questions.

If we're clearing the input on the UI thread, is it impossible for the user to type text on their keyboard before input is cleared? Or there's a small time window in which the user can type text before the UI thread clears the input?

Both actions (typing on the keyboard and pressing the send button) are handled by the operating system and delivered to the application in a form of an event. These events are processed in the event loop running on the UI thread. If the user taps the send button while also pressing a key on the keyboard, it depends on which event occurs first.
Assuming the user has already entered text "Hello", there are exactly two orderings of these two events:

keypress first, tap second

[UI] processing first event (keypress "!")

[UI] keypress event causes a synchronous change of the value of the text input to "Hello!"
[UI] onChangeText RN event is emitted with payload "Hello!"

[UI] processing second event (tap send button)

[UI] text input is cleared immediately using updateProps from a worklet
[UI] we schedule sending a message via runOnJS

[JS] onChangeText is received on the JS thread

[JS] we call setText("Hello!") which triggers a React render, changes text JS state to "Hello!" and sends an event setTextAndSelection with value "Hello!"

[JS] we send a message with current text JS state which is "Hello!"
[UI] setTextAndSelection event is skipped because of eventLag (because we called updateProps before)

tap first, keypress second

[UI] processing first event (tap send button)

[UI] text input is cleared immediately using updateProps from a worklet
[UI] we schedule sending a message via runOnJS

[UI] processing second event (keypress "!")

[UI] keypress event causes a synchronous change of the value of the text input to "!"
[UI] onChangeText RN event is emitted with payload "!"

[JS] we send a message with current text JS state which is "Hello"
[JS] onChangeText is received on the JS thread

[JS] we call setText("!") which triggers a React render, changes text JS state to "!" and sends an event setTextAndSelection with value "!"

[UI] setTextAndSelection event is not skipped because its mostRecentEventCount is based on the onChangeText event which was emitted later than updateProps was called

[UI] the value of the text input is changed to "!"

My opinion here is that we shouldn't worry too much about this case. Imagine we're developing a native iOS app (i.e. there's no separate JS thread, only the main thread and some background threads for async tasks). Our logic would be something like:
when send button is pressed:
  read current value
  clear text input
  schedule sending a message on a background thread

With this code, we also don't have any control over which event comes first (keypress or tap).
Even in the worst-case scenario, we could even obtain the current value of the text input using Reanimated inside a worklet immediately before calling updateProps which clears the text which would be basically the same logic as the pseudocode above.

To rephrase my question, is a race condition between RN's state and textinput's value on the UI thread possible?

Actually, there are at least 3 states:

React state (JS)
RN state (ShadowNode)
UI state (platform)

Obviously, there will be short periods of time where these states are different, but we can't really do anything about it (unless we implement a custom component which stores the current value in a Reanimated shared value). I really hope that the analysis from the first part of the answer guarantees the correct result.

Also, this might be a very basic question but is there only one UI thread and we're using that?
Or we're creating a new UI thread that is different from the "main" UI thread?

"UI thread" basically means the main thread (Thread 1) which is the only thread that you can call UI APIs from (insert views, update views etc.).

Andrew is the Engineer assigned here but I am 👍 to the proposal and also I would like to thank for such well written and explained proposal. Its an inspiration for all the other contributors here.

Agreed with what has been said above, great explanation, thank you @tomekzaw! I've assigned you the issue.

Hi @AndrewGable Actually I will be in charge of implementing @tomekzaw proposal in behalf of Software Mansion. If you could assign me to it.

Didn't get to today, but will tackle tomorrow!

Hi @robertKozik can we get an update here please? Thank you! 🙇♂

Sure, I started implementing it today, so I'll write updates more frequently now

reviewing today and posting next steps!

@robertKozik can we get an update today please? 🙏

Hi @dylanexpensify  I was OOO until today. The PR will be ready for today, worst case scenario tomorrow morning.

Hi! For today: When implementing I encountered the problem with the sent button background. It's not staying in sync with the composer clearing, as it's based on js state, not on ui state of the input. I'll work on it tomorrow, but still post the draft PR for it today

Wooot, great progress here! @robertKozik do you have an ETA for when it will be ready for review? Thank you! 🙇♂

I'm still tacking the problem with the send button. I'm not sure about the ETA on that as I'm having a feeling that I'm close to resolving it.

Sounds good @robertKozik! Keep us updated 😄

Mind giving an update today when you have a spare moment @robertKozik? Ty sir!!

Sure, I think I've overcome all obstacles related to this issue. The performance of the app on the release version is promising, and the bug cannot be reproduced when typing on the release version. I believe I will prepare the pull request (PR) for review by the end of today.

I've encountered minor obstacles during the testing. I need to postpone the PR to tomorrow

PR is under review 🚀

Referring to this slack entry here, it does not look like this issue is resolved with the current fix.

@rojiphil looks like that bug report is 2 days ago.  But the fix was deployed to production today.
Commented in slack if they can still repro it

WIP ⚠

The PR that introduced the bug has been identified. Link to the PR:

The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Determine if we should create a regression test for this bug.

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again

Created a manual request on new dot - https://staging.new.expensify.com/r/5267744771377329

@dylanexpensify should we pay a bug reporting bonus to @ahmdshrif ?

Related, can you summarize the appropriate payments for everyone on this issue?

Yes! Sorry, got stuck with travel yesterday!

Payment summary:

@ahmdshrif Reporting Bonus: $250
@rushatgabhane C+ Review: $8,000

Upwork job is here.

Hello, my name is Francisco Castellanos. I am a systems engineering student and a junior web designer. I saw the job posting and I believe it fits well with my profile. I have completed courses in HTML, CSS, JavaScript, MongoDB, PHP, React, and Node.js.

Reviewed the details for @rushatgabhane. This amount is approved for payment in NewDot.


Add exponential backoff to MQTT client to prevent loop breaking on API failure

This fixes #56 where API errors are causing the MQTT reconnect to fail and the loop to stop - after which the MQTT client will stop receiving updates from the server.
I have implemented exponential backoff (with a cap at 60 seconds) - but I am happy to take feedback if this should be lower/higher as needed.

I am thinking to also add retry to the commands as well, as that's the other one that keeps getting failures.
However that can get a bit trickier, as someone may manually retry the service call - resulting in two calls being instead of one.
Thoughts?
For example:
2022-02-06 20:46:04 DEBUG (Thread-7) [tuya_iot] _on_log: Sending PINGREQ
2022-02-06 20:46:05 DEBUG (Thread-7) [tuya_iot] _on_log: Received PINGRESP
2022-02-06 20:47:05 DEBUG (Thread-7) [tuya_iot] _on_log: Sending PINGREQ
2022-02-06 20:47:05 DEBUG (Thread-7) [tuya_iot] _on_log: Received PINGRESP
2022-02-06 20:47:21 DEBUG (SyncWorker_0) [homeassistant.components.tuya] Sending commands for device 078443178caab57ec4c8: [{'code': <DPCode.SWITCH: 'switch'>, 'value': True}, {'code': <DPCode.MODE: 'mode'>, 'value': 'cold'}]
2022-02-06 20:47:21 DEBUG (SyncWorker_0) [tuya_iot] Request: method = POST,                 url = https://openapi.tuyaus.com/v1.0/devices/078443178caab57ec4c8/commands,                params = None,                body = {'commands': [{'code': <DPCode.SWITCH: 'switch'>, 'value': True}, {'code': <DPCode.MODE: 'mode'>, 'value': 'cold'}]},                t =<PHONE_NUMBER>122
2022-02-06 20:47:26 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [140328344721152] HTTPSConnectionPool(host='openapi.tuyaus.com', port=443): Max retries exceeded with url: /v1.0/devices/078443178caab57ec4c8/commands (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fa0bd581190>: Failed to establish a new connection: [Errno -3] Try again'))
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/urllib3/connection.py", line 174, in _new_conn
    conn = connection.create_connection(
  File "/usr/local/lib/python3.9/site-packages/urllib3/util/connection.py", line 72, in create_connection
    for res in socket.getaddrinfo(host, port, family, socket.SOCK_STREAM):
  File "/usr/local/lib/python3.9/socket.py", line 954, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -3] Try again

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/urllib3/connectionpool.py", line 703, in urlopen
    httplib_response = self._make_request(
  File "/usr/local/lib/python3.9/site-packages/urllib3/connectionpool.py", line 386, in _make_request
    self._validate_conn(conn)
  File "/usr/local/lib/python3.9/site-packages/urllib3/connectionpool.py", line 1040, in _validate_conn
    conn.connect()
  File "/usr/local/lib/python3.9/site-packages/urllib3/connection.py", line 358, in connect
    conn = self._new_conn()
  File "/usr/local/lib/python3.9/site-packages/urllib3/connection.py", line 186, in _new_conn
    raise NewConnectionError(
urllib3.exceptions.NewConnectionError: <urllib3.connection.HTTPSConnection object at 0x7fa0bd581190>: Failed to establish a new connection: [Errno -3] Try again

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/requests/adapters.py", line 440, in send
    resp = conn.urlopen(
  File "/usr/local/lib/python3.9/site-packages/urllib3/connectionpool.py", line 785, in urlopen
    retries = retries.increment(
  File "/usr/local/lib/python3.9/site-packages/urllib3/util/retry.py", line 592, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='openapi.tuyaus.com', port=443): Max retries exceeded with url: /v1.0/devices/078443178caab57ec4c8/commands (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fa0bd581190>: Failed to establish a new connection: [Errno -3] Try again'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 190, in handle_call_service
    await hass.services.async_call(
  File "/usr/src/homeassistant/homeassistant/core.py", line 1630, in async_call
    task.result()
  File "/usr/src/homeassistant/homeassistant/core.py", line 1667, in _execute_service
    await cast(Callable[[ServiceCall], Awaitable[None]], handler.job.target)(
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 204, in handle_service
    await self.hass.helpers.service.entity_service_call(
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 668, in entity_service_call
    future.result()  # pop exception if have
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 921, in async_request_call
    await coro
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 705, in _handle_entity_call
    await result
  File "/usr/src/homeassistant/homeassistant/components/climate/__init__.py", line 470, in async_set_hvac_mode
    await self.hass.async_add_executor_job(self.set_hvac_mode, hvac_mode)
  File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 52, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/usr/src/homeassistant/homeassistant/components/tuya/climate.py", line 275, in set_hvac_mode
    self._send_command(commands)
  File "/usr/src/homeassistant/homeassistant/components/tuya/base.py", line 271, in _send_command
    self.device_manager.send_commands(self.device.id, commands)
  File "/usr/local/lib/python3.9/site-packages/tuya_iot/device.py", line 488, in send_commands
    return self.device_manage.send_commands(device_id, commands)
  File "/usr/local/lib/python3.9/site-packages/tuya_iot/device.py", line 649, in send_commands
    return self.api.post(
  File "/usr/local/lib/python3.9/site-packages/tuya_iot/openapi.py", line 316, in post
    return self.__request("POST", path, None, body)
  File "/usr/local/lib/python3.9/site-packages/tuya_iot/openapi.py", line 266, in __request
    response = self.session.request(
  File "/usr/local/lib/python3.9/site-packages/requests/sessions.py", line 529, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/local/lib/python3.9/site-packages/requests/sessions.py", line 645, in send
    r = adapter.send(request, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/requests/adapters.py", line 519, in send
    raise ConnectionError(e, request=request)
requests.exceptions.ConnectionError: HTTPSConnectionPool(host='openapi.tuyaus.com', port=443): Max retries exceeded with url: /v1.0/devices/078443178caab57ec4c8/commands (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fa0bd581190>: Failed to establish a new connection: [Errno -3] Try again'))

Thoughts?

I would not add retries to that one, instead, catch those exceptions and wrap it into a specific exception for this library. E.g., a TuyaCommandError is something...
It is up to the implementer at that point to retry or not.

Hi @ZephireNZ thanks for the contribution, will merge this PR. @frenck your suggestion is also reasonable, but as most of our integration users don't have technical background, may not know how to handle these exceptions. So @ZephireNZ 's solution is more suitable for now.

@zlinoliver Frenck's feedback was not on this PR, but on the idea of retrying commands. I agree with Frenck. For commands failing, don't retry but instead raise. The user won't see them but HA or any other application integration this SDK will. Let the application decide if they want to retry. Else the application cannot distinguish between a long running command versus errors+retrying under the hood.


Gitea fails to handle webhooks when an emoji is present in the commit name/description

Description
Putting an emoji in a commit (either title or description) makes things go out of sync.
The personal homepage as well a person's "Public Activity" page on their profile won't ever show the push, even after running health checks, restarting the machine, or syncing git hooks.
The commit list on the impacted Git repository is still up-to-date, though.
The repository which exhibited that issue only has a single Discord webhook link.
Shortest possible reproduction:
git commit --allow-empty -m "test 😅"

Relevant gitea.log portion:
2022/10/31 15:01:26 ...n/webhook/webhook.go:631:NotifyPushCommits() [E] [635fe346-11] PrepareWebhooks: CreateHookTask: Error 1366: Incorrect string value: '\xF0\x9F\x98\x85 -...' for column `gitea`.`hook_task`.`payload_content` at row 1
2022/10/31 15:01:26 ...ion/action/action.go:335:NotifyPushCommits() [E] notifyWatchers: insert new actioner: Error 1366: Incorrect string value: '\xF0\x9F\x98\x85\n...' for column `gitea`.`action`.`content` at row 1

Gitea Version
1.18.0+rc0
Git Version
2.30.2
Operating System
Debian 11, Linux 5.10
How are you running Gitea?
The issue happens both on a prebuilt 1.18.0-rc0 binary, or when building with TAGS="bindata" make build
Gitea is being run using systemd (ver. 247).
Database
MySQL

That's a problem with your setup. https://docs.gitea.io/en-us/faq/#why-are-emoji-broken-on-mysql


Enable max retrieval payment per request

For Filecoin Discover, we'd like to use the deal bot to test for free retrieval. In order to do this properly, we'd need to be able to specify a max payment price for retrieval per request (alongside specifying miner and payload CID).
Ideally this would allow us then to distinguish between an "out of funds" error on our Client and a miner attempting to charge non-agreed upon prices for our Client.

@ribasushi

it looks like we anticipated this for storage deals https://github.com/filecoin-project/dealbot/blob/main/tasks/gen.go#L97 but not retrieval deals beyond the payment price configured as default limits in the lotus the bot runs against

Maybe a more general ability to set price per Gb.
Alternatively, use Lotus config for Discover deployment that sets max retrieval price at zero.  JV will see this show up as an error like "cannot agree on price."

this is fixed in #181


Entity Framework Sample Provider example is missing

Describe the bug
This page https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/ef/writing-an-ef-data-provider has a link to a "Entity Framework Sample Provider" under the section "Sample". However, this link redirects to the "Browse code samples"-page. Searching for "entity framework sample provider" does not return any results.
To Reproduce
Steps to reproduce the behavior:

Go to https://docs.microsoft.com/en-us/dotnet/framework/data/adonet/ef/writing-an-ef-data-provider
Scroll down to 'Sample'
Click on the link "Entity Framework Sample Provider"
Search for "entity framework sample provider" in Browse code samples-page.

Expected behavior
I expect it to be easier to find the Entity Framework Sample Provider. Either through a direct link or through searching in the browse code samples-page. At the moment I cannot find it anywhere.

@sstein any ideas on the missing link? Maybe the sample was retired, so we need to remove the link from the doc probably.

Tagging @JeremyLikness here too
This is what the old page looked like: https://web.archive.org/web/20151207060419/https://code.msdn.microsoft.com/windowsdesktop/Entity-Framework-Sample-6a9801d0

All of the EF Core documentation has moved to https://docs.microsoft.com/ef. Here is the current documentation for writing a provider. I'm not sure who owns/maintains the documents referenced, I'll look into a PR that links to the current documentation.

Here is the PR: https://github.com/dotnet/docs/pull/19266


Not usable on OSX

Environment data
.NET Command Line Tools (1.0.0-preview2-003121)
Product Information:
Version:            1.0.0-preview2-003121
Commit SHA-1 hash:  1e9d529bc5
Runtime Environment:
OS Name:     Mac OS X
OS Version:  10.11
OS Platform: Darwin
RID:         osx.10.11-x64
VS Code version:
1.4.0
C# Extension version:
1.3.0
Steps to reproduce
I have a simply folder structure with a global.json in the root referencing several child folders:

global.json
Project.One

project.json

Project.Two

project.json

All projects created using dotnet new
Expected  behavior
Intellisense to work, go to symbol to work, project.json files to be found.
Actual behavior
The C# plugin does not provide intellisense at all if I select the root folder, I have to continuously switch between projects using the "omnisharp: select project" command to select an individual project.json, even then it often doesn't work. All non current projects show huge numbers of errors (e.g. type 'System.Object' could not be found). Go to symbol does not work at all.
I've tried creating new projects from scratch etc. same behaviour...

I belief this is related to: http://stackoverflow.com/questions/38670295/brew-refusing-to-link-openssl/38706045#38706045 as I have a crypto error in the omnisharp logs. I'm continuing to investigate how to fix this without downgrading openssl in brew.
I've already run: sudo install_name_tool -add_rpath /usr/local/opt/openssl/lib /usr/local/share/dotnet/shared/Microsoft.NETCore.App/1.0.0/System.Security.Cryptography.Native.dylib
which fixed issues in dotnet command

I can confirm the crypto error on startup is fixed by running:
sudo install_name_tool -add_rpath /usr/local/opt/openssl/lib ~/.vscode/extensions/ms-vscode.csharp-1.3.0/out/.omnisharp-coreclr/System.Security.Cryptography.Native.dylib

Behaviour is much more reliable now, however, I still have to switch projects to get intellisense.

Once the debugger is downloaded you also need to run the same command on the debuggers version of System.Security.Cryptography.Native.dylib
install_name_tool -add_rpath /usr/local/opt/openssl/lib ~/.vscode/extensions/ms-vscode.csharp-1.3.0/coreclr-debug/debugAdapters/System.Security.Cryptography.Native.dylib

The extension is also not working for me. It fails on installing the .net core debugger.

Note: I resolved my issue with the .net core debugger not installing. I did both:

Downloaded and installed dotnet 1.0.0-preview2-003121 for mac os x from microsoft's website, and

2.sudo install_name_tool -add_rpath /usr/local/opt/openssl/lib /usr/local/share/dotnet/shared/Microsoft.NETCore.App/1.0.0/System.Security.Cryptography.Native.dylib
I am not sure if step 2 was necessary, as I did both before trying to launch VS Code again.

Glad to hear you have it working. The issue is related to https://github.com/dotnet/cli/issues/3964.
Also #676.


KAFKA-3414; Return of MetadataCache.getAliveBrokers should not be mutated by cache updates

Map.values returns DefaultValuesIterable where the default implementation of toSeq is (sadly) toStream. Stream is a lazy collection and it can reflect changes to the underlying map before it's forced.
I verified that the test failed before my change.

Review by @hachikuji and @gwenshap.

Tests passed locally.

+1 good catch


On toml config file library option is not recognised and only db.

** Before submitting a bug report, please ensure you are running the most recent version of osxphotos and that the bug is reproducible on the latest version **
Describe the bug
I read --db option is/is going to be deprecated in favor of --library.
I went to change .toml file config from db to library and noticed library options is not accepted.
To Reproduce
Steps to reproduce the behavior:

What' the full command line you used with osxphotos?
What was the error output?

Expected behavior
Just wondering if the new version which removes --db will consider db or library on the toml file.
Screenshots
> osxphotos export X --library "/Users/Shared/Pictures/iPhoto Shared Library.photoslibrary"  --save-config X.toml --config-only
Saved config file to 'X.toml'
> cat X.toml
[export]
db = "/Users/Shared/Pictures/iPhoto Shared Library.photoslibrary"

If I add manualy library to the toml file:
> cat X.toml
[export]
library = "/Users/Shared/Pictures/iPhoto Shared Library.photoslibrary"
>osxphotos export X   --load-config X.toml --config-only
Error parsing X.toml config file: Unknown option: library =
/Users/Shared/Pictures/iPhoto Shared Library.photoslibrary

Desktop (please complete the following information):

OS: [e.g. which version of macOS]

ProductName:	macOS
ProductVersion:	12.6.9
BuildVersion:	21G726

osxphotos version (osxphotos --version)

osxphotos, version 0.64.3
Python 3.11.5 (main, Aug 24 2023, 15:23:30) [Clang 14.0.0 (clang-14<IP_ADDRESS>)]
macOS 12.6.9, x86_64

Additional context
N/A

Thanks. --db and --library are actually synonyms but the toml file only recognizes the first. There may be other settings like this (but off the top of my head I think that --db is the only one of with a synonym). For clarification, I'm not (yet) deprecating --db only the passing of the database/library as an argument instead of as an option. It caused too much confusion with the export directory for some users. You'll still be able to use --db but that means the same as --library.  Either way, this issue with the toml will need to be fixed. I think --library is more clear than --db.

Thanks for the clarifications. So I'm good.
Yes. Agree. Library is best.

This will be fixed in next release. You can specify library = "X" or db = "X" in the TOML file and it will work fine. If you specify both, you get a warning and the library value is used.


xiaomi_aqara: temperature & humidity unknown (pressure OK)

Hello,
After upgrade to 0.56, temperature & humidity are in "unknown" state... Pressure is displayed.
I try multiple reset of Xiaomi gateway but same issue.
After rollback to 0.55.1, all are displayed.
Home Assistant release (hass --version): 0.56.0
Python release (python3 --version): Python 3.5.3
Component/platform: sensor.xiaomi_aqara

Upgrade to 0.56.1. There was a fix to this issue

Thx @bastshoes. I confirm that this is now fixed in 0.56.1!


Fetch json from database

var myApp = angular.module('myApp', ['fg', 'ngSanitize', 'markdown']);
var myData;
myApp.controller('MyController', function ($scope, mySchema, formService) {
formService.GetCountyForm('', '').then(function (d) {
    myData = d.data[0].FormSchema;
    schema = myData;
    //alert(myData);  Contains json from database
});

$scope.myForm = {
    schema: myData
};

$scope.init = function (value) {
    alert(myData);
    $scope.testInput = myData;
}

});
myApp.value('mySchema',myData);
myApp.factory('formService', function ($http) {
var fac = {};
fac.GetCountyForm = function (stateCode, countyName) {
    //alert($(".select4 :selected").text() + " " + countyName);
    //replacing statecode with statename

    return $http({
        url: '/data/GetFormSchema',
        method: 'POST',
        data: JSON.stringify({stateCode: stateCode, countyName: countyName }),
        headers: { 'content-type': 'application/json' }
    });
}

return fac;

});
My Html looks like

Example Form

                    </fieldset>
                    @*

*@

                        <button type="submit"
                                class="btn btn-primary"
                                ng-class="{ disabled: previewForm.$invalid }">
                            Submit
                        </button>

                </form>

I am getting the json from database, but somehow not getting loaded for the form, I must be missing something, can you help me out with this? Thanks

I have the same problem. How did you solve it (if you did and forgot to post)?


Implements audiobook bookmarks

What's this do?
Implements audiobookmarks
Why are we doing this? (w/ Notion link if applicable)
https://www.notion.so/lyrasis/Add-bookmarks-to-audiobooks-on-iOS-532d22cc549445c6a1ee52da49715a79
How should this be tested? / Do these changes have associated tests?

View audiobook
Add bookmark
Attempt to add bookmark again in same location
navigate to another location, add bookmark
view bookmark tab in TOC, select bookmark
view bookmark tab in TOC, delete bookmark

Dependencies for merging? Releasing to production?
https://github.com/ThePalaceProject/ios-audiobooktoolkit/pull/39
Does this include changes that require a new Palace build for QA?
Yes
Has the application documentation been updated for these changes?
N/A
Did someone actually run this code to verify it works?
@mauricecarrier7

https://user-images.githubusercontent.com/6921353/233681404-d44daa5a-c75e-4446-8233-be2c9fdac993.MP4

It crashes right after [Listen] is tapped, it's either NYPLAudiobookToolkit code is different from what's pushed to the repo, or code in this repo should be different?
Xcode reports NYPLAudiobookToolkit/Core/AudiobookManager.swift line 251:
annotationsDelegate?.fetchBookmarks(for: audiobook.uniqueIdentifier, completion: { bookmarks in

Thread 9: "-[TPPBookCellDelegate fetchBookmarksFor:completion:]: unrecognized selector sent to instance...
and TPPBookCellDelegate.m in this repo doesn't contain fetchBookmarksFor:completion:.

It crashes right after [Listen] is tapped, it's either NYPLAudiobookToolkit code is different from what's pushed to the repo, or code in this repo should be different? Xcode reports NYPLAudiobookToolkit/Core/AudiobookManager.swift line 251:
annotationsDelegate?.fetchBookmarks(for: audiobook.uniqueIdentifier, completion: { bookmarks in

Thread 9: "-[TPPBookCellDelegate fetchBookmarksFor:completion:]: unrecognized selector sent to instance...
and TPPBookCellDelegate.m in this repo doesn't contain fetchBookmarksFor:completion:.

It crashes right after [Listen] is tapped, it's either NYPLAudiobookToolkit code is different from what's pushed to the repo, or code in this repo should be different? Xcode reports NYPLAudiobookToolkit/Core/AudiobookManager.swift line 251:
annotationsDelegate?.fetchBookmarks(for: audiobook.uniqueIdentifier, completion: { bookmarks in

Thread 9: "-[TPPBookCellDelegate fetchBookmarksFor:completion:]: unrecognized selector sent to instance...
and TPPBookCellDelegate.m in this repo doesn't contain fetchBookmarksFor:completion:.

Ah, I think I have a weird caching issue locally that caused me not to see that until I cleaned and recompiled. Please test again.


fix: make response type inferrence distributive

Current dependencies on/for this PR:

main

PR #154   👈

This stack of pull requests is managed by Graphite.


Added quiz app based on CSS

It is a timer based quiz app.

please rename demo to "demo.gif"

please rename demo to "demo.gif"
done