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

[editor] Widget manager should compare widget keys with deep equals instead of string equal #11309

Closed
kittaakos opened this issue Jun 17, 2022 · 1 comment · Fixed by #11450
Closed
Labels
quality issues related to code and application quality shell issues related to the core shell

Comments

@kittaakos
Copy link
Contributor

kittaakos commented Jun 17, 2022

Bug Description:

When opening editors, the opener options is used to generate an identity key for the widget. This is clear and works as expected. However, the property order of the opener options also matters when trying to find an already opened widget.

The following code causes unexpected behavior:

const uri = 'file:///path/to/an/existing/resource.txt';
const options = {
    counter: 0,
    mode: 'reveal',
    preview: false
} as EditorOpenerOptions;
await this.editorManager.open(uri, options);

const sameOptions = {
    ...options,
    preview: options.preview,
    mode: options.mode // change the property order
};
await this.editorManager.open(widget.editor.uri, sameOptions);

My code is here: master...kittaakos:opener-options
Steps to reproduce:

  • Open an editor,
  • Wait 2 sec

See in action:

duplicate.mp4

Do I misuse the APIs? if not, I think Theia should compare keys with deepEqual and try to find the existing widget instead doing Map.get by the JSON options.

Steps to Reproduce:

Additional Information

  • Operating System: macOS 12.3.1
  • Theia Version: 462376a
@msujew msujew added shell issues related to the core shell quality issues related to code and application quality labels Jun 24, 2022
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Jun 29, 2022

Alternatively, something like this library might solve the problem of key-order dependent stringification - it's even already in our node_modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality issues related to code and application quality shell issues related to the core shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants