Schematic Tree fixes #11274
Schematic Tree fixes #11274
Conversation
| @@ -211,6 +212,20 @@ export class SimpleMemoryHost implements Host<{}> { | |||
| const content = this._cache.get(from); | |||
| if (content) { | |||
| this._cache.delete(from); | |||
There was a problem hiding this comment.
This side effect should be moved to the end of the function (next to the set) to avoid inconsistent state if an exception is thrown.
|
|
||
| tree.visit(path => { | ||
| if (!filter(path)) { | ||
| this.delete(path); |
There was a problem hiding this comment.
This will delete files (as actions), no? A FilterTree should contains only actions that are pertained to files that match the pattern, unless you intend to do something else here. But if I have a tree that contains file1 and file2 and return FilterTree(x => x != file2), it would actually delete file2 which is probably not what we mean (and not what FilteredTree would have done).
I don’t see a test on actions of filtertree which would have caught this.
There was a problem hiding this comment.
New host tree specific filter added that does not use delete.
| @@ -98,6 +96,7 @@ export class HostTree implements Tree { | |||
| private _id = _uniqueId++; | |||
There was a problem hiding this comment.
I don’t see this part being fixed from my previous PR; IDs need to be negative to avoid ID clash with VirtualTree.
| @@ -426,3 +404,17 @@ export class HostTree implements Tree { | |||
| ); | |||
| } | |||
| } | |||
|
|
|||
| export class HostCreateTree extends HostTree { | |||
| constructor(host: virtualFs.Host) { | |||
There was a problem hiding this comment.
Should be the same host type as it’s superclass (ReadonlyHost<{}>)
| @@ -16,34 +16,41 @@ export function empty() { | |||
| } | |||
|
|
|||
| export function branch(tree: Tree) { | |||
There was a problem hiding this comment.
Please add TODOs to these functions to remove those VirtualTree hacks for 7.0
| const newBackend = new virtualFs.SimpleMemoryHost(); | ||
| // cast to allow access | ||
| const originalBackend = (tree as FilterHostTree)._backend; | ||
| originalBackend.list(normalize('/')) |
There was a problem hiding this comment.
Are you just filtering the root?
There was a problem hiding this comment.
Indeed. Fixed with additional tests.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Based on #11178