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

Really convert IDs to numbers to use with Map. Re-fetch items that have been marked as changed. #3376

Merged
merged 2 commits into from Dec 20, 2021

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Dec 16, 2021

This PR fixes two bugs. First - call me an idiot, but I used just syntax-sugar conversion instead of Number(x) to convert item IDs (which are numeric) to numbers. I did not notice it during var inspection in the debugger (just '9' vs 9) ... therefore items were not matched and updated properly, the items were never found in the treeData Map.

Second is a race condition: sometimes, especially with lazy-initialized Nodes in NBLS, a query for Children started to initialize the Children, and so a certain nodes/nodeChanged could be received before the vscode even received the first nodes/info response for the node; the node change was then simply ignored. That results in dangling nodes, nodes like 'Please wait...' which are in the tree indefinitely etc.

The fix is to track those pending changes, and delay Promise<TreeItem> completion after a new nodes/info response arrives.

@sdedic sdedic added kind:bug Bug report or fix VSCode Extension [ci] enable VSCode Extension tests labels Dec 16, 2021
@sdedic sdedic added this to the NB13 milestone Dec 16, 2021
@sdedic sdedic requested a review from dbalek December 16, 2021 12:19
@sdedic sdedic self-assigned this Dec 16, 2021
@sdedic
Copy link
Member Author

sdedic commented Dec 16, 2021

I used this PR for a trivial typo fix - see commit 56c1822

@sdedic sdedic added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Dec 16, 2021
@sdedic
Copy link
Member Author

sdedic commented Dec 16, 2021

I am stil hitting an issue with vscode -- when multiple change - getTreeItems are called, the vscode infrastructure errors out. That does not happen when "normal" children re-fetches, like when a new file is created.

@sdedic sdedic removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Dec 20, 2021
@sdedic sdedic merged commit 58d932b into apache:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug Bug report or fix VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants