Suggestion: remove element cache feature, since it's problematic

[t9md]

Description

Related atom/atom#16622

cc @daviwil, @jarle, @nathansobo

Currently command palette cache element.
But it's return cached element when it should not.
Even #105 doesn't not completely solve root issue.

So we have two path.

• Add conditional check where we shouldn't return cache.
• Remove element cache completely

Steps to Reproduce

Essentially this issue is not related to preserveLastSearch setting.
No need to enable preserveLastSearch.
This issue can be reproduced even in same text-edtior.

1. Add following command in your init.js

atom.commands.add('atom-text-editor', {
  'aaa:editor-cmd': () => console.log('hello'),
})
atom.commands.add('atom-text-editor.has-selection', {
  'aaa:editor-cmd-has-selection': () => console.log('hello'),
})

2. launch atom, open editor(e.g. text-editor-component.js).
3. do following

• without selection, open palette, then close
• with selection(select some text), open palette, then close
• without selection, open palette, palette show just AAA: Editor Cmd only(!), then close
• without selection, open palette, boon, you'll see exception Cannot read property 'replaceChild' of null

What's happening?

Brief explanation

Different scope/element return different available command list, when etch update element, it replace old element with new one.
If this replacement was done with cached element on different index, it cause, element move in DOM.
I cannot explain clearly, it seems breaks etch's expectation.

Detail

When number of command element are changed we should not return cached element.
Since active element or scope change available number of command we should not return cached element for different scope.
To achieve that, we need to add scope or element information to determine cache-hit(which is unrealistic I think).

We can observe this with following.

Filter command-palette to show only ['hello1', 'hello2', 'world1', 'world2'] commands

Replace this part

command-palette/lib/command-palette-view.js

Lines 125 to 127 in 60f84dd

	const commandsForActiveElement = atom.commands
	.findCommands({target: this.activeElement})
	.filter(command => showHiddenCommands === !!command.hiddenInCommandPalette)

    const commandsForActiveElement = atom.commands
        .findCommands({target: this.activeElement})
        .filter(command => showHiddenCommands === !!command.hiddenInCommandPalette)
        .filter(command => ['hello1', 'hello2', 'world1', 'world2'].includes(command.name.split(':')[1]))

Add also id to each cached element for easy to observe

li.id = nextElementID++
this.elementCache.get(item).set(queryKey, li)

Define command in workspace and text-editor scope

atom.commands.add('atom-workspace', {
  'workspace:hello1': () => console.log('hello'),
  'workspace:hello2': () => console.log('hello'),
  'workspace:world1': () => console.log('world'),
  'workspace:world2': () => console.log('world'),
})

atom.commands.add('atom-text-editor', {
  'aaa:editor-cmd': () => console.log('hello'),
  'editor:hello1': () => console.log('hello'),
  'editor:hello2': () => console.log('hello'),
  'editor:world1': () => console.log('world'),
  'editor:world2': () => console.log('world'),
})

insert debug log on replace child event of select-list's ListItemView::update

  destroy () {
    console.log('destroyed', this.element.id, this.element); // debug
    this.element.remove()
    this.domEventsDisposable.dispose()
  }

  update (props) {
    this.element.removeEventListener('mousedown', this.mouseDown)
    this.element.removeEventListener('mouseup', this.mouseUp)
    this.element.removeEventListener('click', this.didClick)

    console.log('replaceChild', props.element, this.element); // debug

1. open palette in editor shows 8 commands, two time to populate cache for easy to comparison
2. open palette in setting-view shows just workspace:hello1

Why? workspace:hello1 is not returned from cache, since it's selected state was change from false to true).
Other item should be displayed are destroyed.

Expected behavior: [What you expect to happen]

No exception.

Actual behavior: [What actually happens]

Throw exception palette become unusable.

Reproduces how often: [What percentage of the time does it reproduce?]

100%

Versions

master, atom-beta

Additional Information

When I create this PR, I have to invalidate cache, since element are reordered, it's essentially same reason what's happening here.
#103