From 970b221309c7a745ccdd5eb02eedbc715e321d3a Mon Sep 17 00:00:00 2001 From: David Michon Date: Tue, 17 Dec 2024 16:30:59 -0800 Subject: [PATCH] Enhance LookupByPath, Fix win32 paths in webpack-workspace-resolve-plugin (#5050) * [lookup-by-path] Add size,entries,get,has,removeItem Add more support for delimiter overrides per method. Exclude `undefined` and `null` from allowed values to type parameter * [workspace-resolve] Fix win32 path formatting. Also tap hooks earlier to avoid sequencing issues. --------- Co-authored-by: David Michon --- .../lookup-enhancements_2024-12-17-22-37.json | 10 + .../lookup-enhancements_2024-12-17-22-42.json | 10 + common/reviews/api/lookup-by-path.api.md | 32 +- libraries/lookup-by-path/src/LookupByPath.ts | 217 ++++++++++- .../src/test/LookupByPath.test.ts | 366 ++++++++++++++++++ .../src/KnownPackageDependenciesPlugin.ts | 6 +- .../src/WorkspaceLayoutCache.ts | 23 +- .../src/WorkspaceResolvePlugin.ts | 17 +- .../src/test/createResolveForTests.ts | 2 +- 9 files changed, 642 insertions(+), 41 deletions(-) create mode 100644 common/changes/@rushstack/lookup-by-path/lookup-enhancements_2024-12-17-22-37.json create mode 100644 common/changes/@rushstack/webpack-workspace-resolve-plugin/lookup-enhancements_2024-12-17-22-42.json diff --git a/common/changes/@rushstack/lookup-by-path/lookup-enhancements_2024-12-17-22-37.json b/common/changes/@rushstack/lookup-by-path/lookup-enhancements_2024-12-17-22-37.json new file mode 100644 index 00000000000..cdfccfa59d4 --- /dev/null +++ b/common/changes/@rushstack/lookup-by-path/lookup-enhancements_2024-12-17-22-37.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/lookup-by-path", + "comment": "Update all methods to accept optional override delimiters. Add `size`, `entries(), `get()`, `has()`, `removeItem()`. Make class iterable.\nExplicitly exclude `undefined` and `null` from the allowed types for the type parameter `TItem`.", + "type": "minor" + } + ], + "packageName": "@rushstack/lookup-by-path" +} \ No newline at end of file diff --git a/common/changes/@rushstack/webpack-workspace-resolve-plugin/lookup-enhancements_2024-12-17-22-42.json b/common/changes/@rushstack/webpack-workspace-resolve-plugin/lookup-enhancements_2024-12-17-22-42.json new file mode 100644 index 00000000000..4311b7fa133 --- /dev/null +++ b/common/changes/@rushstack/webpack-workspace-resolve-plugin/lookup-enhancements_2024-12-17-22-42.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/webpack-workspace-resolve-plugin", + "comment": "Fix a bug with path handling on Windows. Tap hooks earlier to ensure that these plugins run before builtin behavior.", + "type": "patch" + } + ], + "packageName": "@rushstack/webpack-workspace-resolve-plugin" +} \ No newline at end of file diff --git a/common/reviews/api/lookup-by-path.api.md b/common/reviews/api/lookup-by-path.api.md index 6fcf473ecd5..694afe0e223 100644 --- a/common/reviews/api/lookup-by-path.api.md +++ b/common/reviews/api/lookup-by-path.api.md @@ -5,31 +5,43 @@ ```ts // @beta -export interface IPrefixMatch { +export interface IPrefixMatch { index: number; lastMatch?: IPrefixMatch; value: TItem; } // @beta -export interface IReadonlyLookupByPath { - findChildPath(childPath: string): TItem | undefined; +export interface IReadonlyLookupByPath extends Iterable<[string, TItem]> { + [Symbol.iterator](query?: string, delimiter?: string): IterableIterator<[string, TItem]>; + entries(query?: string, delimiter?: string): IterableIterator<[string, TItem]>; + findChildPath(childPath: string, delimiter?: string): TItem | undefined; findChildPathFromSegments(childPathSegments: Iterable): TItem | undefined; - findLongestPrefixMatch(query: string): IPrefixMatch | undefined; - groupByChild(infoByPath: Map): Map>; + findLongestPrefixMatch(query: string, delimiter?: string): IPrefixMatch | undefined; + get(query: string, delimiter?: string): TItem | undefined; + groupByChild(infoByPath: Map, delimiter?: string): Map>; + has(query: string, delimiter?: string): boolean; + get size(): number; } // @beta -export class LookupByPath implements IReadonlyLookupByPath { +export class LookupByPath implements IReadonlyLookupByPath { + [Symbol.iterator](query?: string, delimiter?: string): IterableIterator<[string, TItem]>; constructor(entries?: Iterable<[string, TItem]>, delimiter?: string); + clear(): this; + deleteItem(query: string, delimeter?: string): boolean; readonly delimiter: string; - findChildPath(childPath: string): TItem | undefined; + entries(query?: string, delimiter?: string): IterableIterator<[string, TItem]>; + findChildPath(childPath: string, delimiter?: string): TItem | undefined; findChildPathFromSegments(childPathSegments: Iterable): TItem | undefined; - findLongestPrefixMatch(query: string): IPrefixMatch | undefined; - groupByChild(infoByPath: Map): Map>; + findLongestPrefixMatch(query: string, delimiter?: string): IPrefixMatch | undefined; + get(key: string, delimiter?: string): TItem | undefined; + groupByChild(infoByPath: Map, delimiter?: string): Map>; + has(key: string, delimiter?: string): boolean; static iteratePathSegments(serializedPath: string, delimiter?: string): Iterable; - setItem(serializedPath: string, value: TItem): this; + setItem(serializedPath: string, value: TItem, delimiter?: string): this; setItemFromSegments(pathSegments: Iterable, value: TItem): this; + get size(): number; } ``` diff --git a/libraries/lookup-by-path/src/LookupByPath.ts b/libraries/lookup-by-path/src/LookupByPath.ts index c4b7d01366d..63868c0a677 100644 --- a/libraries/lookup-by-path/src/LookupByPath.ts +++ b/libraries/lookup-by-path/src/LookupByPath.ts @@ -4,11 +4,12 @@ /** * A node in the path trie used in LookupByPath */ -interface IPathTrieNode { +interface IPathTrieNode { /** * The value that exactly matches the current relative path */ value: TItem | undefined; + /** * Child nodes by subfolder */ @@ -20,6 +21,7 @@ interface IPrefixEntry { * The prefix that was matched */ prefix: string; + /** * The index of the first character after the matched prefix */ @@ -31,15 +33,17 @@ interface IPrefixEntry { * * @beta */ -export interface IPrefixMatch { +export interface IPrefixMatch { /** * The item that matched the prefix */ value: TItem; + /** * The index of the first character after the matched prefix */ index: number; + /** * The last match found (with a shorter prefix), if any */ @@ -51,7 +55,7 @@ export interface IPrefixMatch { * * @beta */ -export interface IReadonlyLookupByPath { +export interface IReadonlyLookupByPath extends Iterable<[string, TItem]> { /** * Searches for the item associated with `childPath`, or the nearest ancestor of that path that * has an associated item. @@ -65,7 +69,7 @@ export interface IReadonlyLookupByPath { * trie.findChildPath('foo/bar/baz'); // returns 2 * ``` */ - findChildPath(childPath: string): TItem | undefined; + findChildPath(childPath: string, delimiter?: string): TItem | undefined; /** * Searches for the item for which the recorded prefix is the longest matching prefix of `query`. @@ -81,7 +85,7 @@ export interface IReadonlyLookupByPath { * trie.findLongestPrefixMatch('foo/bar/baz'); // returns { item: 2, index: 7 } * ``` */ - findLongestPrefixMatch(query: string): IPrefixMatch | undefined; + findLongestPrefixMatch(query: string, delimiter?: string): IPrefixMatch | undefined; /** * Searches for the item associated with `childPathSegments`, or the nearest ancestor of that path that @@ -98,6 +102,56 @@ export interface IReadonlyLookupByPath { */ findChildPathFromSegments(childPathSegments: Iterable): TItem | undefined; + /** + * Determines if an entry exists exactly at the specified path. + * + * @returns `true` if an entry exists at the specified path, `false` otherwise + */ + has(query: string, delimiter?: string): boolean; + + /** + * Retrieves the entry that exists exactly at the specified path, if any. + * + * @returns The entry that exists exactly at the specified path, or `undefined` if no entry exists. + */ + get(query: string, delimiter?: string): TItem | undefined; + + /** + * Gets the number of entries in this trie. + * + * @returns The number of entries in this trie. + */ + get size(): number; + + /** + * Iterates over the entries in this trie. + * + * @param query - An optional query. If specified only entries that start with the query will be returned. + * + * @returns An iterator over the entries under the specified query (or the root if no query is specified). + * @remarks + * Keys in the returned iterator use the provided delimiter to join segments. + * Iteration order is not specified. + * @example + * ```ts + * const trie = new LookupByPath([['foo', 1], ['foo/bar', 2]]); + * [...trie.entries(undefined, ',')); // returns [['foo', 1], ['foo,bar', 2]] + * ``` + */ + entries(query?: string, delimiter?: string): IterableIterator<[string, TItem]>; + + /** + * Iterates over the entries in this trie. + * + * @param query - An optional query. If specified only entries that start with the query will be returned. + * + * @returns An iterator over the entries under the specified query (or the root if no query is specified). + * @remarks + * Keys in the returned iterator use the provided delimiter to join segments. + * Iteration order is not specified. + */ + [Symbol.iterator](query?: string, delimiter?: string): IterableIterator<[string, TItem]>; + /** * Groups the provided map of info by the nearest entry in the trie that contains the path. If the path * is not found in the trie, the info is ignored. @@ -106,7 +160,7 @@ export interface IReadonlyLookupByPath { * * @param infoByPath - The info to be grouped, keyed by path */ - groupByChild(infoByPath: Map): Map>; + groupByChild(infoByPath: Map, delimiter?: string): Map>; } /** @@ -129,16 +183,22 @@ export interface IReadonlyLookupByPath { * ``` * @beta */ -export class LookupByPath implements IReadonlyLookupByPath { +export class LookupByPath implements IReadonlyLookupByPath { /** * The delimiter used to split paths */ public readonly delimiter: string; + /** * The root node of the trie, corresponding to the path '' */ private readonly _root: IPathTrieNode; + /** + * The number of entries in this trie. + */ + private _size: number; + /** * Constructs a new `LookupByPath` * @@ -151,6 +211,7 @@ export class LookupByPath implements IReadonlyLookupByPath { }; this.delimiter = delimiter ?? '/'; + this._size = 0; if (entries) { for (const [path, item] of entries) { @@ -201,14 +262,52 @@ export class LookupByPath implements IReadonlyLookupByPath { } } + /** + * {@inheritdoc IReadonlyLookupByPath} + */ + public get size(): number { + return this._size; + } + + /** + * Deletes all entries from this `LookupByPath` instance. + * + * @returns this, for chained calls + */ + public clear(): this { + this._root.value = undefined; + this._root.children = undefined; + this._size = 0; + return this; + } + /** * Associates the value with the specified serialized path. * If a value is already associated, will overwrite. * * @returns this, for chained calls */ - public setItem(serializedPath: string, value: TItem): this { - return this.setItemFromSegments(LookupByPath.iteratePathSegments(serializedPath, this.delimiter), value); + public setItem(serializedPath: string, value: TItem, delimiter: string = this.delimiter): this { + return this.setItemFromSegments(LookupByPath.iteratePathSegments(serializedPath, delimiter), value); + } + + /** + * Deletes an item if it exists. + * @param query - The path to the item to delete + * @param delimeter - Optional override delimeter for parsing the query + * @returns `true` if the item was found and deleted, `false` otherwise + * @remarks + * If the node has children with values, they will be retained. + */ + public deleteItem(query: string, delimeter: string = this.delimiter): boolean { + const node: IPathTrieNode | undefined = this._findNodeAtPrefix(query, delimeter); + if (node?.value !== undefined) { + node.value = undefined; + this._size--; + return true; + } + + return false; } /** @@ -235,6 +334,9 @@ export class LookupByPath implements IReadonlyLookupByPath { } node = child; } + if (node.value === undefined) { + this._size++; + } node.value = value; return this; @@ -243,15 +345,18 @@ export class LookupByPath implements IReadonlyLookupByPath { /** * {@inheritdoc IReadonlyLookupByPath} */ - public findChildPath(childPath: string): TItem | undefined { - return this.findChildPathFromSegments(LookupByPath.iteratePathSegments(childPath, this.delimiter)); + public findChildPath(childPath: string, delimiter: string = this.delimiter): TItem | undefined { + return this.findChildPathFromSegments(LookupByPath.iteratePathSegments(childPath, delimiter)); } /** * {@inheritdoc IReadonlyLookupByPath} */ - public findLongestPrefixMatch(query: string): IPrefixMatch | undefined { - return this._findLongestPrefixMatch(LookupByPath._iteratePrefixes(query, this.delimiter)); + public findLongestPrefixMatch( + query: string, + delimiter: string = this.delimiter + ): IPrefixMatch | undefined { + return this._findLongestPrefixMatch(LookupByPath._iteratePrefixes(query, delimiter)); } /** @@ -281,11 +386,30 @@ export class LookupByPath implements IReadonlyLookupByPath { /** * {@inheritdoc IReadonlyLookupByPath} */ - public groupByChild(infoByPath: Map): Map> { + public has(key: string, delimiter: string = this.delimiter): boolean { + const match: IPrefixMatch | undefined = this.findLongestPrefixMatch(key, delimiter); + return match?.index === key.length; + } + + /** + * {@inheritdoc IReadonlyLookupByPath} + */ + public get(key: string, delimiter: string = this.delimiter): TItem | undefined { + const match: IPrefixMatch | undefined = this.findLongestPrefixMatch(key, delimiter); + return match?.index === key.length ? match.value : undefined; + } + + /** + * {@inheritdoc IReadonlyLookupByPath} + */ + public groupByChild( + infoByPath: Map, + delimiter: string = this.delimiter + ): Map> { const groupedInfoByChild: Map> = new Map(); for (const [path, info] of infoByPath) { - const child: TItem | undefined = this.findChildPath(path); + const child: TItem | undefined = this.findChildPath(path, delimiter); if (child === undefined) { continue; } @@ -300,6 +424,45 @@ export class LookupByPath implements IReadonlyLookupByPath { return groupedInfoByChild; } + /** + * {@inheritdoc IReadonlyLookupByPath} + */ + public *entries(query?: string, delimiter: string = this.delimiter): IterableIterator<[string, TItem]> { + let root: IPathTrieNode | undefined; + if (query) { + root = this._findNodeAtPrefix(query, delimiter); + if (!root) { + return; + } + } else { + root = this._root; + } + + const stack: [string, IPathTrieNode][] = [[query ?? '', root]]; + while (stack.length > 0) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const [prefix, node] = stack.pop()!; + if (node.value !== undefined) { + yield [prefix, node.value]; + } + if (node.children) { + for (const [segment, child] of node.children) { + stack.push([prefix ? `${prefix}${delimiter}${segment}` : segment, child]); + } + } + } + } + + /** + * {@inheritdoc IReadonlyLookupByPath} + */ + public [Symbol.iterator]( + query?: string, + delimiter: string = this.delimiter + ): IterableIterator<[string, TItem]> { + return this.entries(query, delimiter); + } + /** * Iterates through progressively longer prefixes of a given string and returns as soon * as the number of candidate items that match the prefix are 1 or 0. @@ -340,4 +503,28 @@ export class LookupByPath implements IReadonlyLookupByPath { return best; } + + /** + * Finds the node at the specified path, or `undefined` if no node was found. + * + * @param query - The path to the node to search for + * @returns The trie node at the specified path, or `undefined` if no node was found + */ + private _findNodeAtPrefix( + query: string, + delimiter: string = this.delimiter + ): IPathTrieNode | undefined { + let node: IPathTrieNode = this._root; + for (const { prefix } of LookupByPath._iteratePrefixes(query, delimiter)) { + if (!node.children) { + return undefined; + } + const child: IPathTrieNode | undefined = node.children.get(prefix); + if (!child) { + return undefined; + } + node = child; + } + return node; + } } diff --git a/libraries/lookup-by-path/src/test/LookupByPath.test.ts b/libraries/lookup-by-path/src/test/LookupByPath.test.ts index 3d3aa341080..369de61d4f2 100644 --- a/libraries/lookup-by-path/src/test/LookupByPath.test.ts +++ b/libraries/lookup-by-path/src/test/LookupByPath.test.ts @@ -26,6 +26,339 @@ describe(LookupByPath.iteratePathSegments.name, () => { }); }); +describe('size', () => { + it('returns 0 for an empty tree', () => { + expect(new LookupByPath().size).toEqual(0); + }); + + it('returns the number of nodes for a non-empty tree', () => { + const lookup: LookupByPath = new LookupByPath([['foo', 1]]); + expect(lookup.size).toEqual(1); + lookup.setItem('bar', 2); + expect(lookup.size).toEqual(2); + lookup.setItem('bar', 4); + expect(lookup.size).toEqual(2); + lookup.setItem('bar/baz', 1); + expect(lookup.size).toEqual(3); + lookup.setItem('foo/bar/qux/quux', 1); + expect(lookup.size).toEqual(4); + }); +}); + +describe(LookupByPath.prototype.get.name, () => { + it('returns undefined for an empty tree', () => { + expect(new LookupByPath().get('foo')).toEqual(undefined); + }); + + it('returns the matching node for a trivial tree', () => { + expect(new LookupByPath([['foo', 1]]).get('foo')).toEqual(1); + }); + + it('returns undefined for non-matching paths in a single-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['bar', 2], + ['baz', 3] + ]); + + expect(tree.get('buzz')).toEqual(undefined); + expect(tree.get('foo/bar')).toEqual(undefined); + }); + + it('returns the matching node for a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.get('foo')).toEqual(1); + expect(tree.get('foo/bar')).toEqual(2); + expect(tree.get('foo/bar/baz')).toEqual(3); + + expect(tree.get('foo')).toEqual(1); + expect(tree.get('foo,bar', ',')).toEqual(2); + expect(tree.get('foo\0bar\0baz', '\0')).toEqual(3); + }); + + it('returns undefined for non-matching paths in a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.get('foo/baz')).toEqual(undefined); + expect(tree.get('foo/bar/baz/qux')).toEqual(undefined); + }); +}); + +describe(LookupByPath.prototype.has.name, () => { + it('returns false for an empty tree', () => { + expect(new LookupByPath().has('foo')).toEqual(false); + }); + + it('returns true for the matching node in a trivial tree', () => { + expect(new LookupByPath([['foo', 1]]).has('foo')).toEqual(true); + }); + + it('returns false for non-matching paths in a single-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['bar', 2], + ['baz', 3] + ]); + + expect(tree.has('buzz')).toEqual(false); + expect(tree.has('foo/bar')).toEqual(false); + }); + + it('returns true for the matching node in a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.has('foo')).toEqual(true); + expect(tree.has('foo/bar')).toEqual(true); + expect(tree.has('foo/bar/baz')).toEqual(true); + + expect(tree.has('foo')).toEqual(true); + expect(tree.has('foo,bar', ',')).toEqual(true); + expect(tree.has('foo\0bar\0baz', '\0')).toEqual(true); + }); + + it('returns false for non-matching paths in a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.has('foo/baz')).toEqual(false); + expect(tree.has('foo/bar/baz/qux')).toEqual(false); + }); +}); + +describe(LookupByPath.prototype.clear.name, () => { + it('clears an empty tree', () => { + const tree = new LookupByPath(); + tree.clear(); + expect(tree.size).toEqual(0); + }); + + it('clears a single-layer tree', () => { + const tree = new LookupByPath([['foo', 1]]); + expect(tree.size).toEqual(1); + tree.clear(); + expect(tree.size).toEqual(0); + }); + + it('clears a multi-layer tree', () => { + const tree = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + expect(tree.size).toEqual(3); + tree.clear(); + expect(tree.size).toEqual(0); + }); + + it('clears a tree with custom delimiters', () => { + const tree = new LookupByPath( + [ + ['foo,bar', 1], + ['foo,bar,baz', 2] + ], + ',' + ); + expect(tree.size).toEqual(2); + tree.clear(); + expect(tree.size).toEqual(0); + }); +}); + +describe(LookupByPath.prototype.entries.name, () => { + it('returns an empty iterator for an empty tree', () => { + const tree = new LookupByPath(); + const result = [...tree]; + expect(result).toEqual([]); + }); + + it('returns an iterator for a single-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['bar', 2], + ['baz', 3] + ]); + + const result = [...tree]; + expect(result.length).toEqual(tree.size); + expect(Object.fromEntries(result)).toEqual({ + foo: 1, + bar: 2, + baz: 3 + }); + }); + + it('returns an iterator for a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + const result = [...tree]; + expect(result.length).toEqual(tree.size); + expect(Object.fromEntries(result)).toEqual({ + foo: 1, + 'foo/bar': 2, + 'foo/bar/baz': 3 + }); + }); + + it('only includes non-empty nodes', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo/bar/baz', 1], + ['foo/bar/baz/qux/quux', 2] + ]); + + const result = [...tree]; + expect(result.length).toEqual(tree.size); + expect(Object.fromEntries(result)).toEqual({ + 'foo/bar/baz': 1, + 'foo/bar/baz/qux/quux': 2 + }); + }); + + it('returns an iterator for a tree with custom delimiters', () => { + const tree: LookupByPath = new LookupByPath( + [ + ['foo,bar', 1], + ['foo,bar,baz', 2] + ], + ',' + ); + + const result = [...tree]; + expect(result.length).toEqual(tree.size); + expect(Object.fromEntries(result)).toEqual({ + 'foo,bar': 1, + 'foo,bar,baz': 2 + }); + }); + + it('returns an iterator for a subtree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3], + ['bar', 4], + ['bar/baz', 5] + ]); + + const result = [...tree.entries('foo')]; + expect(result.length).toEqual(3); + expect(Object.fromEntries(result)).toEqual({ + foo: 1, + 'foo/bar': 2, + 'foo/bar/baz': 3 + }); + }); + + it('returns an iterator for a subtree with custom delimiters', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo/bar', 1], + ['foo/bar/baz', 2], + ['bar/baz', 3] + ]); + + const result = [...tree.entries('foo', ',')]; + expect(result.length).toEqual(2); + expect(Object.fromEntries(result)).toEqual({ + 'foo,bar': 1, + 'foo,bar,baz': 2 + }); + }); +}); + +describe(LookupByPath.prototype.deleteItem.name, () => { + it('returns false for an empty tree', () => { + expect(new LookupByPath().deleteItem('foo')).toEqual(false); + }); + + it('deletes the matching node in a trivial tree', () => { + const tree = new LookupByPath([['foo', 1]]); + expect(tree.deleteItem('foo')).toEqual(true); + expect(tree.size).toEqual(0); + expect(tree.get('foo')).toEqual(undefined); + }); + + it('returns false for non-matching paths in a single-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['bar', 2], + ['baz', 3] + ]); + + expect(tree.deleteItem('buzz')).toEqual(false); + expect(tree.size).toEqual(3); + }); + + it('deletes the matching node in a single-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['bar', 2], + ['baz', 3] + ]); + + expect(tree.deleteItem('bar')).toEqual(true); + expect(tree.size).toEqual(2); + expect(tree.get('bar')).toEqual(undefined); + }); + + it('deletes the matching node in a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.deleteItem('foo/bar')).toEqual(true); + expect(tree.size).toEqual(2); + expect(tree.get('foo/bar')).toEqual(undefined); + expect(tree.get('foo/bar/baz')).toEqual(3); // child nodes are retained + }); + + it('returns false for non-matching paths in a multi-layer tree', () => { + const tree: LookupByPath = new LookupByPath([ + ['foo', 1], + ['foo/bar', 2], + ['foo/bar/baz', 3] + ]); + + expect(tree.deleteItem('foo/baz')).toEqual(false); + expect(tree.size).toEqual(3); + }); + + it('handles custom delimiters', () => { + const tree: LookupByPath = new LookupByPath( + [ + ['foo,bar', 1], + ['foo,bar,baz', 2] + ], + ',' + ); + + expect(tree.deleteItem('foo\0bar', '\0')).toEqual(true); + expect(tree.size).toEqual(1); + expect(tree.get('foo\0bar', '\0')).toEqual(undefined); + expect(tree.get('foo\0bar\0baz', '\0')).toEqual(2); // child nodes are retained + }); +}); + describe(LookupByPath.prototype.findChildPath.name, () => { it('returns empty for an empty tree', () => { expect(new LookupByPath().findChildPath('foo')).toEqual(undefined); @@ -101,6 +434,8 @@ describe(LookupByPath.prototype.findChildPath.name, () => { ); expect(tree.findChildPath('foo/bar,baz')).toEqual(2); + expect(tree.findChildPath('foo,bar,baz', ',')).toEqual(1); + expect(tree.findChildPath('foo\0bar\0baz', '\0')).toEqual(1); expect(tree.findChildPath('foo,bar/baz')).toEqual(undefined); expect(tree.findChildPathFromSegments(['foo', 'bar', 'baz'])).toEqual(1); }); @@ -180,6 +515,37 @@ describe(LookupByPath.prototype.groupByChild.name, () => { expect(lookup.groupByChild(infoByPath)).toEqual(expected); }); + it('groups items by the closest group that contains the file path with custom delimiter', () => { + const customLookup: LookupByPath = new LookupByPath( + [ + ['foo,bar', 'bar'], + ['foo,bar,baz', 'baz'] + ], + ',' + ); + + const infoByPath: Map = new Map([ + ['foo\0bar', 'bar'], + ['foo\0bar\0baz', 'baz'], + ['foo\0bar\0baz\0qux', 'qux'], + ['foo\0bar\0baz\0qux\0quux', 'quux'] + ]); + + const expected: Map> = new Map([ + ['bar', new Map([['foo\0bar', 'bar']])], + [ + 'baz', + new Map([ + ['foo\0bar\0baz', 'baz'], + ['foo\0bar\0baz\0qux', 'qux'], + ['foo\0bar\0baz\0qux\0quux', 'quux'] + ]) + ] + ]); + + expect(customLookup.groupByChild(infoByPath, '\0')).toEqual(expected); + }); + it('ignores items that do not exist in the lookup', () => { const infoByPath: Map = new Map([ ['foo', 'foo'], diff --git a/webpack/webpack-workspace-resolve-plugin/src/KnownPackageDependenciesPlugin.ts b/webpack/webpack-workspace-resolve-plugin/src/KnownPackageDependenciesPlugin.ts index 041d15751f0..d5ad882d338 100644 --- a/webpack/webpack-workspace-resolve-plugin/src/KnownPackageDependenciesPlugin.ts +++ b/webpack/webpack-workspace-resolve-plugin/src/KnownPackageDependenciesPlugin.ts @@ -56,7 +56,11 @@ export class KnownPackageDependenciesPlugin { let scope: IPrefixMatch | undefined = cache.contextForPackage.get(descriptionFileData); if (!scope) { - return callback(new Error(`Expected context for ${request.descriptionFileRoot}`)); + scope = cache.contextLookup.findLongestPrefixMatch(path); + if (!scope) { + return callback(new Error(`Expected context for ${request.descriptionFileRoot}`)); + } + cache.contextForPackage.set(descriptionFileData, scope); } let dependency: IPrefixMatch | undefined; diff --git a/webpack/webpack-workspace-resolve-plugin/src/WorkspaceLayoutCache.ts b/webpack/webpack-workspace-resolve-plugin/src/WorkspaceLayoutCache.ts index 166099f7e41..0f2d988b634 100644 --- a/webpack/webpack-workspace-resolve-plugin/src/WorkspaceLayoutCache.ts +++ b/webpack/webpack-workspace-resolve-plugin/src/WorkspaceLayoutCache.ts @@ -150,9 +150,8 @@ export class WorkspaceLayoutCache { public get descriptionFileRoot(): string { if (!this._descriptionFileRoot) { - this._descriptionFileRoot = `${basePath}${ - normalizeToPlatform?.(this._serialized.root) ?? this._serialized.root - }`; + const merged: string = `${basePath}${this._serialized.root}`; + this._descriptionFileRoot = normalizeToPlatform?.(merged) ?? merged; } return this._descriptionFileRoot; } @@ -183,8 +182,14 @@ export class WorkspaceLayoutCache { const resolveContext: ResolveContext = new ResolveContext(serialized); resolveContexts.push(resolveContext); - const descriptionFileRoot: string = resolveContext.descriptionFileRoot; - contextLookup.setItem(descriptionFileRoot, resolveContext); + contextLookup.setItemFromSegments( + concat( + // All paths in the cache file are platform-agnostic + LookupByPath.iteratePathSegments(basePath, '/'), + LookupByPath.iteratePathSegments(serialized.root, '/') + ), + resolveContext + ); // Handle nested package.json files. These may modify some properties, but the dependency resolution // will match the original package root. Typically these are used to set the `type` field to `module`. @@ -192,9 +197,11 @@ export class WorkspaceLayoutCache { for (const file of serialized.dirInfoFiles) { contextLookup.setItemFromSegments( concat( - // Root is normalized to platform slashes - LookupByPath.iteratePathSegments(descriptionFileRoot, resolverPathSeparator), - // Subpaths are platform-agnostic + // All paths in the cache file are platform-agnostic + concat( + LookupByPath.iteratePathSegments(basePath, '/'), + LookupByPath.iteratePathSegments(serialized.root, '/') + ), LookupByPath.iteratePathSegments(file, '/') ), resolveContext diff --git a/webpack/webpack-workspace-resolve-plugin/src/WorkspaceResolvePlugin.ts b/webpack/webpack-workspace-resolve-plugin/src/WorkspaceResolvePlugin.ts index 432a736216c..6cbd3be6ca9 100644 --- a/webpack/webpack-workspace-resolve-plugin/src/WorkspaceResolvePlugin.ts +++ b/webpack/webpack-workspace-resolve-plugin/src/WorkspaceResolvePlugin.ts @@ -49,17 +49,22 @@ export class WorkspaceResolvePlugin implements WebpackPluginInstance { resolveOptions.plugins ??= []; resolveOptions.plugins.push( // Optimize identifying the package.json file for the issuer - new KnownDescriptionFilePlugin(cache, 'parsed-resolve', 'described-resolve'), + new KnownDescriptionFilePlugin(cache, 'before-parsed-resolve', 'described-resolve'), // Optimize locating the installed dependencies of the current package - new KnownPackageDependenciesPlugin(cache, 'raw-module', 'resolve-as-module'), + new KnownPackageDependenciesPlugin(cache, 'before-raw-module', 'resolve-as-module'), // Optimize loading the package.json file for the destination package (bare specifier) - new KnownDescriptionFilePlugin(cache, 'resolve-as-module', 'resolve-in-package'), + new KnownDescriptionFilePlugin(cache, 'before-resolve-as-module', 'resolve-in-package'), // Optimize loading the package.json file for the destination package (relative path) - new KnownDescriptionFilePlugin(cache, 'relative', 'described-relative'), + new KnownDescriptionFilePlugin(cache, 'before-relative', 'described-relative'), // Optimize locating and loading nested package.json for a directory - new KnownDescriptionFilePlugin(cache, 'undescribed-existing-directory', 'existing-directory', true), + new KnownDescriptionFilePlugin( + cache, + 'before-undescribed-existing-directory', + 'existing-directory', + true + ), // Optimize locating and loading nested package.json for a file - new KnownDescriptionFilePlugin(cache, 'undescribed-raw-file', 'raw-file') + new KnownDescriptionFilePlugin(cache, 'before-undescribed-raw-file', 'raw-file') ); return resolveOptions; diff --git a/webpack/webpack-workspace-resolve-plugin/src/test/createResolveForTests.ts b/webpack/webpack-workspace-resolve-plugin/src/test/createResolveForTests.ts index 2a3ea512b92..bc47fa52857 100644 --- a/webpack/webpack-workspace-resolve-plugin/src/test/createResolveForTests.ts +++ b/webpack/webpack-workspace-resolve-plugin/src/test/createResolveForTests.ts @@ -35,7 +35,7 @@ export function createResolveForTests( const cache: WorkspaceLayoutCache = new WorkspaceLayoutCache({ cacheData: { - basePath: `${separator}workspace${separator}`, + basePath: `/workspace/`, contexts: [ { root: 'a',