chore(client): ts migration #50521
chore(client): ts migration #50521
Conversation
github-actions
bot
added
the
platform: learn
|
👀 Review this PR in a CodeSee Review Map View the CodeSee Map of this changeLegend
|
| @@ -26,12 +40,13 @@ class WorkerExecutor { | |||
| if (e.data?.type === 'contentLoaded') { | |||
| resolve(newWorker); | |||
| } | |||
| // do something else if not contentLoaded? | |||
There was a problem hiding this comment.
i'm in search of some advice. getworker is supposed to return a worker by calling createworker. but if createworker doesn’t get ‘contentloaded’ or has an error, then it won’t return a worker. Right now the failure to get ‘contentloaded’ would result in the call to getWorker inside execute never resolving and the whole thing getting stuck. Typescript is forcing me to address this more directly and so I’ve gone down this path but I wanted some feedback if it seems reasonable?
There was a problem hiding this comment.
You can see it get stuck by changing the test. If the mockworker doesn't call 'contentloaded', the whole thing will hang till jest shuts it down.
There was a problem hiding this comment.
i know @ojeytonwilliams had worked on this code before. is it cool to tag you for advice here?
There was a problem hiding this comment.
I'm afraid I've never understood this code particularly well, so I can't give particularly useful advice.
All I can say is that, despite being confusing, this code works well. So, if you need to use @ts-expect-error to keep TS from complaining, that's totally fine.
|
Consider opening PRs as draft (not the same as "convert to draft" after the fact) if you have something in the works. It helps cut the notification noise. I appreciate your understanding. |
Actually I never knew that was an option. Will do. |
Sembauke
added
the
other: decayed
|
Hey @jmetev1, I will be closing this pull-request as it has been abandoned. Feel free to revisit/re-open when you feel like it. |
Checklist:
mainbranch of freeCodeCamp.Closes #XXXXX