fix(torch): cannot be turned off

[Sec-ant]

This PR is targeting branch at #383 not master.

• Augment types to avoid using @ts-ignore.
• Fix torch cannot be turned off by user, or when exits, see #380 .

[gruhn]

Thanks :pray:

Implementa a task queue to mitigate race conditions

What's the race condition scenario here? Usually I would prefer to let Vue's reactive system take care of the synchronization business, since it's more declarative.

[Sec-ant]

Thanks 🙏

Implementa a task queue to mitigate race conditions

What's the race condition scenario here? Usually I would prefer to let Vue's reactive system take care of the synchronization business, since it's more declarative.

Yeah this PR is a little opinionated. I think we have to stop the camera and then start it to apply updated constraints (including torch)? Starting & stoping are both async actions with some delays. The task queue is basically waiting for the previous task (start or stop) to finish before doing the current one. The orignal code will drop the action if the previous action hasn't finished. I didn't touch the Vue side of the code, so it's probably ok to still use Vue for state management on the component level?

[gruhn]

I think we have to stop the camera and then start it to apply updated constraints (including torch)?

Exactly, in principle torch can be turned on without stopping the camera but at the time I implemented it, it couldn't be turned off without stopping the camera. So I thought it's more consistent and less surprising to users, if updating the torch simply always causes a stop and restart.

The orignal code will drop the action if the previous action hasn't finished.

Ah, you mean for example a stop-action could be lost, if a start-action is still going on? In theory this scenario should be covered here:

https://github.com/gruhn/vue-qrcode-reader/blob/892bdc028864543446504de9c2b95ac2b5d0f869/src/components/QrcodeStream.vue#L114-L122

This async-stuff is really hard to think about. I should probably implement some tests for this :S

[Sec-ant]

@gruhn I've updated my branch to only include the minimal fix for the torch issue brought by @ts-ignore. Refactoring is discarded.

As this PR doesn't trigger the netlify bot, I manually deployed a demo page: https://65200f2e3d590a0cd1a30f7e--magenta-marigold-7846d9.netlify.app/

But I still think we should reconsider the design of start and stop. I noticed if I click the torch on/off button (the lightning icon in the demo) very quickly, the torch cannot be turned off again. The error is caused by the confliction of load() and play() (added in #378)

My mind is very reactish so I'm afraid I can't help much on the Vue side.

[gruhn]

Thanks for going the extra mile :pray:

But I still think we should reconsider the design of start and stop.

Definitely, I'll merge your concept, just trying to understand what the actual race condition is and why it occurs. Can you created a dedicated PR?

I noticed if I click the torch on/off button (the lightning icon in the demo) very quickly, the torch cannot be turned off again. The error is caused by the confliction of load() and play() (added in https://github.com/gruhn/vue-qrcode-reader/pull/378)

Ah ok interesting. I see play is also returning a Promise. Would it help to await that too?

In the meantime I'll try to implement some race condition tests.

[Sec-ant]

Can you created a dedicated PR?

Do you mean this current one or the one before force push (with the task queue implemented)?

[gruhn]

Can you created a dedicated PR?

Do you mean this current one or the one before force push (with the task queue implemented)?

I suggest to create a dedicated PR for the task queue, so we can merge initial torch fix independently.