Update to rxjs 6.3.3. Use prettier to format. #2
Conversation
|
Thank you for your kind words and the long overdue update to the latest RxJS! 🙇♂️ That's a lot of changes, it may take some time until I find the time to review them, but I'm looking forward to it. Of course you're free to do whatever you want with my open-source code, no need to ask for permission. |
| withLatestFrom, | ||
| take, | ||
| sample | ||
| } from 'rxjs/operators'; | ||
|
|
||
| /* Graphics */ |
There was a problem hiding this comment.
I think overall the structure of the file has suffered. Before it had sections marked with comments (/* Graphics */...), now the structure of the file is a bit confusing.
| map, | ||
| scan, | ||
| withLatestFrom, | ||
| take, |
There was a problem hiding this comment.
take seems to be unused?
And in what order are the imports?
| @@ -0,0 +1,4 @@ | |||
| { | |||
There was a problem hiding this comment.
Thanks for the rc file, but right now it assumes a globally installed prettier and knowledge on how to run it? Could you add prettier as a dependency and add a script to package.json as well?
| context.font = '24px Courier New'; | ||
| context.fillText('rxjs breakout', canvas.width / 2, canvas.height / 2 - 24); | ||
| } | ||
| const INITIAL_OBJECTS = { |
There was a problem hiding this comment.
Grouping the constants together at the start of the file seems a good idea, although I'm personally torn between "group by game components" and "group by language constructs" for this example project.
| distinctUntilChanged() | ||
| ); | ||
|
|
||
| // paddle$.pipe(take(600)).subscribe(x => console.log(x)); |
|
|
||
| // paddle$.pipe(take(600)).subscribe(x => console.log(x)); | ||
|
|
||
| const object$ = ticker$.pipe( |
There was a problem hiding this comment.
objects$? But that's just nitpicking, the singular is okay ;)
| { | ||
| "semi": true, | ||
| "singleQuote": true | ||
| } |
There was a problem hiding this comment.
Adding printWidth: 120 should prevent some of the weirder line breaks after formatting?
|
|
||
| /** Game Loop **/ | ||
| const game = combineLatest(ticker$, paddle$, object$) | ||
| // .pipe(sample(TICKER_INTERVAL)) |
There was a problem hiding this comment.
Is the sampling not necessary anymore?
| let audio; | ||
| let beeper = new Subject(); | ||
|
|
||
| // Cant play sounds until after an interaction |
|
|
||
| // Cant play sounds until after an interaction | ||
| fromEvent(document, 'keyup') | ||
| .toPromise() |
There was a problem hiding this comment.
Is this executed after each keyup? Can/should we add a take(1) or something like that here?
There was a problem hiding this comment.
Or does this complete after the first event?
There was a problem hiding this comment.
Yeah, that looks suspicious (I thought you needed complete for a toPromise to fulfill) , but it seemed to make a warning go away and bring sound back.
Manuel - you don't know how much your work here has been an inspiration for at least 2 years now. I finally am doing some stuff with RxJS for real and want to use this as a demo. I'd like your permission, and if you'd give a mention that'd be great.
It's currently deployed here with some of my cosmetics. I'd like to use this as an example in my Rx-wrapper project Antares which aims to make event driven programming easier.