[ZEPPELIN-6472] Replace deprecated toPromise() with firstValueFrom in CompletionService - #5309
Conversation
… CompletionService - Use firstValueFrom over the unchanged filter/take(1)/map pipeline in provideCompletionItems, replacing the RxJS 7-deprecated toPromise() - Import firstValueFrom from rxjs - Remove leftover debug console.log in onCompletion Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tbonelee
left a comment
There was a problem hiding this comment.
LGTM. Semantically equivalent to the previous .toPromise() since the pipeline already ends with take(1), and the empty-stream EmptyError case is unreachable because completionItem$ is never completed. Nice console.log cleanup too. Thanks!
|
|
||
| return that.completionItem$ | ||
| .pipe( | ||
| return firstValueFrom( |
There was a problem hiding this comment.
Non-blocking, and really just answering the question you already raised in the PR description: agreed that the empty-stream EmptyError difference is only theoretical here, since completionItem$ is never completed, so I don't think a defaultValue is necessary for this PR. If you'd still prefer the extra safety net, firstValueFrom(..., { defaultValue: { suggestions: [] } }) would cover it, but I'm fine either way.
|
Merged into master |
What is this PR for?
CompletionService(zeppelin-web-angular/src/app/services/completion.service.ts) powers Monaco editor code completion for the New UI. InbindMonacoCompletion(), theprovideCompletionItemsreturn path converts a one-shot completion Observable into a Promise using.toPromise(), which is deprecated in RxJS 7 and scheduled for removal in RxJS 8. The RxJS-recommended replacement isfirstValueFrom/lastValueFrom.This PR:
.toPromise()withfirstValueFrom(...), wrapping the unchangedcompletionItem$.pipe(filter(...), take(1), map(...))expression. Because the pipeline already ends withtake(1), it emits a single value, sofirstValueFromis semantically equivalent and Monaco'sprovideCompletionItemsaccepts aPromisereturn.firstValueFromfrom therxjsroot (merged into the existingSubjectimport line; therxjs/operatorsimport is unchanged).console.log('on receive!', data.id)debug statement inonCompletion()that printed on everyCOMPLETION_LISTmessage.No other behavioral changes.
What type of PR is it?
Improvement
Todos
.toPromise()withfirstValueFromover the unchanged filter/take(1)/map pipelinefirstValueFromfromrxjsconsole.loginonCompletion()What is the Jira issue?
ZEPPELIN-6472
How should this be tested?
This repository's frontend has no unit-test infrastructure, so verification is via lint plus a production build:
Both pass.
Questions:
.toPromise()resolvedundefinedon completion whilefirstValueFromrejects withEmptyError. In the happy path behavior is identical because theSubjectis never explicitly completed; the empty-stream difference is a theoretical edge case. AdefaultValuecould be added if reviewers prefer.