Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion items getters are not expanded in reconciliator #15125

Open
krassowski opened this issue Sep 15, 2023 · 0 comments
Open

Completion items getters are not expanded in reconciliator #15125

krassowski opened this issue Sep 15, 2023 · 0 comments

Comments

@krassowski
Copy link
Member

Description

The code in the reconciliator uses spread syntax to copy completion item so that it can add a fallback resolve method from provider. This destroys getters.

const items = reply.items.map(el => ({
...el,
resolve: this._resolveFactory(provider, el)
}));

It is possible to workaround downstream by defining getters as enumerable but this is suboptimal as these will be immediately materialized (so potentially missing opportunity to be populated from resolve request).

Reproduce

  1. Create a provider which returns a an item with insertText getter
  2. Execute completion
  3. See that insertText is undefined and de-duplication does not work

Expected behavior

  1. Use Object.assign instead of spread syntax
  2. We should test that an instance with getter can be used as completion item
  3. Maybe we should avoid creating a copy of completion object in reconciliator in the first place, and internally return a new wrapper object like:
    {
       item: el,
       resolveFallback: this._resolveFactory(provider, el)
    } 

This has a benefit that we could later extend it with renderDocumentation: () => Element so that both LSP markdown-based and kernel sphinx-based docs are rendered properly.

Or, really do we need a factory for a resolve method in the first place? Could we just pass the provider around?

I think that the intent was to later delete the resolve method to mark object as resolved. I do not have a full solution yet.

Context

  • JupyterLab version: 4.0.6
@krassowski krassowski added bug pkg:completer status:Needs Triage Applied to new issues that need triage labels Sep 15, 2023
krassowski added a commit to krassowski/jupyterlab-lsp that referenced this issue Sep 15, 2023
@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants