diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index fe0e5eda94c..15829295403 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -168,6 +168,16 @@ export default class AssetGraph extends ContentGraph { addNode(node: AssetGraphNode): NodeId { this.hash = null; + let existing = this.getNodeByContentKey(node.id); + if (existing != null) { + invariant(existing.type === node.type); + // $FlowFixMe[incompatible-type] Checked above + // $FlowFixMe[prop-missing] + existing.value = node.value; + let existingId = this.getNodeIdByContentKey(node.id); + this.updateNode(existingId, existing); + return existingId; + } return super.addNodeByContentKey(node.id, node); } diff --git a/packages/core/core/src/ContentGraph.js b/packages/core/core/src/ContentGraph.js index 0be5dd1c6c0..9cfdad0e651 100644 --- a/packages/core/core/src/ContentGraph.js +++ b/packages/core/core/src/ContentGraph.js @@ -1,31 +1,32 @@ // @flow strict-local +import type {ContentKey, NodeId} from './types'; import Graph, {type GraphOpts} from './Graph'; -import type {ContentKey, Node, NodeId} from './types'; import nullthrows from 'nullthrows'; -export type SerializedContentGraph< - TNode: Node, - TEdgeType: string | null = null, -> = {| +export type SerializedContentGraph = {| ...GraphOpts, _contentKeyToNodeId: Map, + _nodeIdToContentKey: Map, |}; export default class ContentGraph< - TNode: Node, + TNode, TEdgeType: string | null = null, > extends Graph { _contentKeyToNodeId: Map; + _nodeIdToContentKey: Map; constructor(opts: ?SerializedContentGraph) { if (opts) { - let {_contentKeyToNodeId, ...rest} = opts; + let {_contentKeyToNodeId, _nodeIdToContentKey, ...rest} = opts; super(rest); this._contentKeyToNodeId = _contentKeyToNodeId; + this._nodeIdToContentKey = _nodeIdToContentKey; } else { super(); this._contentKeyToNodeId = new Map(); + this._nodeIdToContentKey = new Map(); } } @@ -41,21 +42,19 @@ export default class ContentGraph< return { ...super.serialize(), _contentKeyToNodeId: this._contentKeyToNodeId, + _nodeIdToContentKey: this._nodeIdToContentKey, }; } addNodeByContentKey(contentKey: ContentKey, node: TNode): NodeId { - if (!this.hasContentKey(contentKey)) { - let nodeId = super.addNode(node); - this._contentKeyToNodeId.set(contentKey, nodeId); - return nodeId; - } else { - let existingNodeId = this.getNodeIdByContentKey(contentKey); - let existingNode = nullthrows(this.getNodeByContentKey(contentKey)); - existingNode.value = node.value; - this.updateNode(existingNodeId, existingNode); - return existingNodeId; + if (this.hasContentKey(contentKey)) { + throw new Error('Graph already has content key ' + contentKey); } + + let nodeId = super.addNode(node); + this._contentKeyToNodeId.set(contentKey, nodeId); + this._nodeIdToContentKey.set(nodeId, contentKey); + return nodeId; } getNodeByContentKey(contentKey: ContentKey): ?TNode { @@ -76,9 +75,11 @@ export default class ContentGraph< return this._contentKeyToNodeId.has(contentKey); } - removeNode(nodeId: NodeId) { + removeNode(nodeId: NodeId): void { this._assertHasNodeId(nodeId); - this._contentKeyToNodeId.delete(nullthrows(this.getNode(nodeId)).id); + let contentKey = nullthrows(this._nodeIdToContentKey.get(nodeId)); + this._contentKeyToNodeId.delete(contentKey); + this._nodeIdToContentKey.delete(nodeId); super.removeNode(nodeId); } } diff --git a/packages/core/core/src/Graph.js b/packages/core/core/src/Graph.js index ffca83fefc3..fe37bfa5023 100644 --- a/packages/core/core/src/Graph.js +++ b/packages/core/core/src/Graph.js @@ -1,7 +1,7 @@ // @flow strict-local import {toNodeId, fromNodeId} from './types'; -import type {Edge, Node, NodeId} from './types'; +import type {Edge, NodeId} from './types'; import type {TraversalActions, GraphVisitor} from '@parcel/types'; import assert from 'assert'; @@ -16,7 +16,7 @@ export type GraphOpts = {| export const ALL_EDGE_TYPES = '@@all_edge_types'; -export default class Graph { +export default class Graph { nodes: Map; inboundEdges: AdjacencyList; outboundEdges: AdjacencyList; diff --git a/packages/core/core/src/public/MutableBundleGraph.js b/packages/core/core/src/public/MutableBundleGraph.js index 14796780991..a0a0143adc0 100644 --- a/packages/core/core/src/public/MutableBundleGraph.js +++ b/packages/core/core/src/public/MutableBundleGraph.js @@ -88,16 +88,15 @@ export default class MutableBundleGraph extends BundleGraph entryAssetId: resolved.id, }; - let bundleGroupNode = { - id: getBundleGroupId(bundleGroup), - type: 'bundle_group', - value: bundleGroup, - }; + let bundleGroupKey = getBundleGroupId(bundleGroup); + let bundleGroupNodeId = this.#graph._graph.hasContentKey(bundleGroupKey) + ? this.#graph._graph.getNodeIdByContentKey(bundleGroupKey) + : this.#graph._graph.addNodeByContentKey(bundleGroupKey, { + id: bundleGroupKey, + type: 'bundle_group', + value: bundleGroup, + }); - let bundleGroupNodeId = this.#graph._graph.addNodeByContentKey( - bundleGroupNode.id, - bundleGroupNode, - ); let dependencyNodeId = this.#graph._graph.getNodeIdByContentKey( dependencyNode.id, ); diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 6d37fdf3a13..53cf6ec1c8b 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -255,13 +255,6 @@ export type Edge = {| type: TEdgeType, |}; -export interface Node { - id: ContentKey; - +type: string; - // $FlowFixMe - value: any; -} - export type AssetNode = {| id: ContentKey, +type: 'asset', diff --git a/packages/core/core/test/ContentGraph.test.js b/packages/core/core/test/ContentGraph.test.js index 046500c480e..3cb0e66d95d 100644 --- a/packages/core/core/test/ContentGraph.test.js +++ b/packages/core/core/test/ContentGraph.test.js @@ -7,7 +7,7 @@ describe('ContentGraph', () => { it('should addNodeByContentKey if no node exists with the content key', () => { let graph = new ContentGraph(); - const node = {id: 'contentKey', type: 'mynode', value: ' 1'}; + const node = {}; const nodeId1 = graph.addNodeByContentKey('contentKey', node); @@ -16,29 +16,23 @@ describe('ContentGraph', () => { assert.deepEqual(graph.getNodeByContentKey('contentKey'), node); }); - it('should update the node through addNodeByContentKey if a node with the content key exists', () => { + it('should throw if a node with the content key already exists', () => { let graph = new ContentGraph(); - const node1 = {id: 'contentKey', value: '1', type: 'mynode'}; - const node2 = {id: 'contentKey', value: '2', type: 'mynode'}; + graph.addNodeByContentKey('contentKey', {}); - const nodeId1 = graph.addNodeByContentKey('contentKey', node1); - const nodeId2 = graph.addNodeByContentKey('contentKey', node2); - - assert.deepEqual(graph.getNode(nodeId1), node1); - assert(graph.hasContentKey('contentKey')); - - assert.equal(nodeId1, nodeId2); - assert.deepEqual(graph.getNode(nodeId2), node2); + assert.throws(() => { + graph.addNodeByContentKey('contentKey', {}); + }, /already has content key/); }); it('should remove the content key from graph when node is removed', () => { let graph = new ContentGraph(); - const node1 = {id: 'contentKey', value: '1', type: 'mynode'}; + const node1 = {}; const nodeId1 = graph.addNodeByContentKey('contentKey', node1); - assert.deepEqual(graph.getNode(nodeId1), node1); + assert.equal(graph.getNode(nodeId1), node1); assert(graph.hasContentKey('contentKey')); graph.removeNode(nodeId1); diff --git a/packages/core/core/test/Graph.test.js b/packages/core/core/test/Graph.test.js index 179d5baed1b..f311d01996b 100644 --- a/packages/core/core/test/Graph.test.js +++ b/packages/core/core/test/Graph.test.js @@ -15,7 +15,7 @@ describe('Graph', () => { it('addNode should add a node to the graph', () => { let graph = new Graph(); - let node = {id: 'do not use', type: 'mynode', value: 'a'}; + let node = {}; let id = graph.addNode(node); assert.equal(graph.nodes.get(id), node); }); @@ -52,7 +52,7 @@ describe('Graph', () => { it("errors when adding an edge to a node that doesn't exist", () => { let graph = new Graph(); - let node = graph.addNode({id: 'foo', type: 'mynode', value: null}); + let node = graph.addNode({}); assert.throws(() => { graph.addEdge(node, toNodeId(-1)); }, /"to" node '-1' not found/); @@ -60,7 +60,7 @@ describe('Graph', () => { it("errors when adding an edge from a node that doesn't exist", () => { let graph = new Graph(); - let node = graph.addNode({id: 'foo', type: 'mynode', value: null}); + let node = graph.addNode({}); assert.throws(() => { graph.addEdge(toNodeId(-1), node); }, /"from" node '-1' not found/); @@ -68,24 +68,24 @@ describe('Graph', () => { it('hasNode should return a boolean based on whether the node exists in the graph', () => { let graph = new Graph(); - let node = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); + let node = graph.addNode({}); assert(graph.hasNode(node)); assert(!graph.hasNode(toNodeId(-1))); }); it('addEdge should add an edge to the graph', () => { let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: null}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: null}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); graph.addEdge(nodeA, nodeB); assert(graph.hasEdge(nodeA, nodeB)); }); it('isOrphanedNode should return true or false if the node is orphaned or not', () => { let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); graph.addEdge(nodeA, nodeB); graph.addEdge(nodeA, nodeC, 'edgetype'); assert(graph.isOrphanedNode(nodeA)); @@ -100,10 +100,10 @@ describe('Graph', () => { // / // c let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); + let nodeD = graph.addNode('d'); graph.addEdge(nodeA, nodeB); graph.addEdge(nodeA, nodeD); graph.addEdge(nodeB, nodeC); @@ -138,13 +138,13 @@ describe('Graph', () => { // f let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); - let nodeE = graph.addNode({id: 'e', type: 'mynode', value: 'e'}); - let nodeF = graph.addNode({id: 'f', type: 'mynode', value: 'f'}); - let nodeG = graph.addNode({id: 'g', type: 'mynode', value: 'g'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); + let nodeD = graph.addNode('d'); + let nodeE = graph.addNode('e'); + let nodeF = graph.addNode('f'); + let nodeG = graph.addNode('g'); graph.addEdge(nodeA, nodeB); graph.addEdge(nodeA, nodeC); @@ -181,13 +181,13 @@ describe('Graph', () => { // f let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); - let nodeE = graph.addNode({id: 'e', type: 'mynode', value: 'e'}); - let nodeF = graph.addNode({id: 'f', type: 'mynode', value: 'f'}); - let nodeG = graph.addNode({id: 'g', type: 'mynode', value: 'g'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); + let nodeD = graph.addNode('d'); + let nodeE = graph.addNode('e'); + let nodeF = graph.addNode('f'); + let nodeG = graph.addNode('g'); graph.setRootNodeId(nodeA); graph.addEdge(nodeA, nodeB); @@ -216,11 +216,11 @@ describe('Graph', () => { // \ / | // e ----- let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); - let nodeE = graph.addNode({id: 'e', type: 'mynode', value: 'e'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); + let nodeD = graph.addNode('d'); + let nodeE = graph.addNode('e'); graph.setRootNodeId(nodeA); graph.addEdge(nodeA, nodeB); @@ -248,8 +248,8 @@ describe('Graph', () => { it('removing a node with only one inbound edge does not cause it to be removed as an orphan', () => { let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); graph.setRootNodeId(nodeA); graph.addEdge(nodeA, nodeB); @@ -266,13 +266,13 @@ describe('Graph', () => { it("replaceNodeIdsConnectedTo should update a node's downstream nodes", () => { let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); graph.addEdge(nodeA, nodeB); graph.addEdge(nodeA, nodeC); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); + let nodeD = graph.addNode('d'); graph.replaceNodeIdsConnectedTo(nodeA, [nodeB, nodeD]); assert(graph.hasNode(nodeA)); @@ -287,10 +287,10 @@ describe('Graph', () => { it('traverses along edge types if a filter is given', () => { let graph = new Graph(); - let nodeA = graph.addNode({id: 'a', type: 'mynode', value: 'a'}); - let nodeB = graph.addNode({id: 'b', type: 'mynode', value: 'b'}); - let nodeC = graph.addNode({id: 'c', type: 'mynode', value: 'c'}); - let nodeD = graph.addNode({id: 'd', type: 'mynode', value: 'd'}); + let nodeA = graph.addNode('a'); + let nodeB = graph.addNode('b'); + let nodeC = graph.addNode('c'); + let nodeD = graph.addNode('d'); graph.addEdge(nodeA, nodeB, 'edgetype'); graph.addEdge(nodeA, nodeD);