Skip to content

[ZEPPELIN-6472] Replace deprecated toPromise() with firstValueFrom in CompletionService#5309

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6472
Open

[ZEPPELIN-6472] Replace deprecated toPromise() with firstValueFrom in CompletionService#5309
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6472

Conversation

@kimyenac

@kimyenac kimyenac commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

CompletionService (zeppelin-web-angular/src/app/services/completion.service.ts) powers Monaco editor code completion for the New UI. In bindMonacoCompletion(), the provideCompletionItems return 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 is firstValueFrom/lastValueFrom.

This PR:

  • Replaces the trailing .toPromise() with firstValueFrom(...), wrapping the unchanged completionItem$.pipe(filter(...), take(1), map(...)) expression. Because the pipeline already ends with take(1), it emits a single value, so firstValueFrom is semantically equivalent and Monaco's provideCompletionItems accepts a Promise return.
  • Imports firstValueFrom from the rxjs root (merged into the existing Subject import line; the rxjs/operators import is unchanged).
  • Removes a leftover unconditional console.log('on receive!', data.id) debug statement in onCompletion() that printed on every COMPLETION_LIST message.

No other behavioral changes.

What type of PR is it?

Improvement

Todos

  • - Replace .toPromise() with firstValueFrom over the unchanged filter/take(1)/map pipeline
  • - Import firstValueFrom from rxjs
  • - Remove leftover debug console.log in onCompletion()

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:

cd zeppelin-web-angular
npm run lint && npm run build:angular

Both pass.

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No. Edge case worth noting: with no completion match, the old .toPromise() resolved undefined on completion while firstValueFrom rejects with EmptyError. In the happy path behavior is identical because the Subject is never explicitly completed; the empty-stream difference is a theoretical edge case. A defaultValue could be added if reviewers prefer.
  • Does this needs documentation? No

… 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 tbonelee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

@tbonelee tbonelee Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants