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

Update without bundling for a 'simple' non-dependency related change #6514

Merged
merged 17 commits into from Oct 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/core/src/AssetGraph.js
Expand Up @@ -114,6 +114,7 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
hash: ?string;
envCache: Map<string, Environment>;
symbolPropagationRan: boolean;
safeToIncrementallyBundle: boolean = true;

constructor(opts: ?AssetGraphOpts) {
if (opts) {
Expand Down
19 changes: 17 additions & 2 deletions packages/core/core/src/BundleGraph.js
Expand Up @@ -26,6 +26,7 @@ import type {
} from './types';
import type AssetGraph from './AssetGraph';
import type {ProjectPath} from './projectPath';
import {nodeFromAsset} from './AssetGraph';

import assert from 'assert';
import invariant from 'assert';
Expand Down Expand Up @@ -1883,8 +1884,7 @@ export default class BundleGraph {
otherGraphIdToThisNodeId.set(otherNodeId, existingNodeId);

let existingNode = nullthrows(this._graph.getNode(existingNodeId));
// Merge symbols, recompute dep.exluded based on that

// Merge symbols, recompute dep.excluded based on that
if (existingNode.type === 'asset') {
invariant(otherNode.type === 'asset');
existingNode.usedSymbols = new Set([
Expand Down Expand Up @@ -1936,6 +1936,21 @@ export default class BundleGraph {
.some(n => n.type === 'root');
}

/**
* Update the asset in a Bundle Graph and clear the associated Bundle hash.
*/
updateAsset(asset: Asset) {
this._graph.updateNode(
this._graph.getNodeIdByContentKey(asset.id),
nodeFromAsset(asset),
);
let bundles = this.getBundlesWithAsset(asset);
for (let bundle of bundles) {
// the bundle content will change with a modified asset
this._bundleContentHashes.delete(bundle.id);
}
}

getEntryRoot(projectRoot: FilePath, target: Target): FilePath {
let cached = this._targetEntryRoots.get(target.distDir);
if (cached != null) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/core/src/UncommittedAsset.js
Expand Up @@ -92,7 +92,7 @@ export default class UncommittedAsset {
}

/*
* Prepares the asset for being serialized to the cache by commiting its
* Prepares the asset for being serialized to the cache by committing its
* content and map of the asset to the cache.
*/
async commit(pipelineKey: string): Promise<void> {
Expand Down
99 changes: 90 additions & 9 deletions packages/core/core/src/requests/AssetGraphRequest.js
Expand Up @@ -49,7 +49,11 @@ type AssetGraphRequestInput = {|
requestedAssetIds?: Set<string>,
|};

type AssetGraphRequestResult = {|
type AssetGraphRequestResult = AssetGraphBuilderResult & {|
previousAssetGraphHash: ?string,
|};

type AssetGraphBuilderResult = {|
assetGraph: AssetGraph,
changedAssets: Map<string, Asset>,
assetRequests: Array<AssetGroup>,
Expand All @@ -63,11 +67,7 @@ type RunInput = {|
type AssetGraphRequest = {|
id: string,
+type: 'asset_graph_request',
run: RunInput => Async<{|
assetGraph: AssetGraph,
changedAssets: Map<string, Asset>,
assetRequests: Array<AssetGroup>,
|}>,
run: RunInput => Async<AssetGraphRequestResult>,
input: AssetGraphRequestInput,
|};

Expand All @@ -80,8 +80,23 @@ export default function createAssetGraphRequest(
run: async input => {
let prevResult =
await input.api.getPreviousResult<AssetGraphRequestResult>();
let previousAssetGraphHash = prevResult?.assetGraph.getHash();

let builder = new AssetGraphBuilder(input, prevResult);
return builder.build();
let assetGraphRequest = await await builder.build();

// early break for incremental bundling if production or flag is off;
if (
!input.options.shouldBundleIncrementally ||
input.options.mode === 'production'
) {
assetGraphRequest.assetGraph.safeToIncrementallyBundle = false;
}

return {
...assetGraphRequest,
previousAssetGraphHash,
};
},
input,
};
Expand Down Expand Up @@ -109,7 +124,7 @@ export class AssetGraphBuilder {

constructor(
{input, api, options}: RunInput,
prevResult: ?AssetGraphRequestResult,
prevResult: ?AssetGraphBuilderResult,
) {
let {
entries,
Expand All @@ -120,6 +135,7 @@ export class AssetGraphBuilder {
shouldBuildLazily,
} = input;
let assetGraph = prevResult?.assetGraph ?? new AssetGraph();
assetGraph.safeToIncrementallyBundle = true;
assetGraph.setRootConnections({
entries,
assetGroups,
Expand All @@ -138,7 +154,7 @@ export class AssetGraphBuilder {
this.queue = new PromiseQueue();
}

async build(): Promise<AssetGraphRequestResult> {
async build(): Promise<AssetGraphBuilderResult> {
let errors = [];
let rootNodeId = nullthrows(
this.assetGraph.rootNodeId,
Expand Down Expand Up @@ -235,6 +251,7 @@ export class AssetGraphBuilder {
if (this.shouldBuildLazily) {
let node = nullthrows(this.assetGraph.getNode(nodeId));
let childNode = nullthrows(this.assetGraph.getNode(childNodeId));

if (node.type === 'asset' && childNode.type === 'dependency') {
if (this.requestedAssetIds.has(node.value.id)) {
node.requested = true;
Expand Down Expand Up @@ -284,6 +301,7 @@ export class AssetGraphBuilder {
assetSymbolsInverse = new Map<Symbol, Set<Symbol>>();
for (let [s, {local}] of assetSymbols) {
let set = assetSymbolsInverse.get(local);

if (!set) {
set = new Set();
assetSymbolsInverse.set(local, set);
Expand Down Expand Up @@ -512,6 +530,7 @@ export class AssetGraphBuilder {
}

let local = outgoingDepSymbols.get(s)?.local;

if (local == null) {
// Caused by '*' => '*', already handled
continue;
Expand Down Expand Up @@ -917,11 +936,34 @@ export class AssetGraphBuilder {
}

async runEntryRequest(input: ProjectPath) {
let prevEntries = this.assetGraph.safeToIncrementallyBundle
? this.assetGraph
.getEntryAssets()
.map(asset => asset.id)
.sort()
: [];

let request = createEntryRequest(input);
let result = await this.api.runRequest<ProjectPath, EntryResult>(request, {
force: true,
});
this.assetGraph.resolveEntry(request.input, result.entries, request.id);

if (this.assetGraph.safeToIncrementallyBundle) {
let currentEntries = this.assetGraph
.getEntryAssets()
.map(asset => asset.id)
.sort();
let didEntriesChange =
prevEntries.length !== currentEntries.length ||
prevEntries.every(
(entryId, index) => entryId === currentEntries[index],
);

if (didEntriesChange) {
this.assetGraph.safeToIncrementallyBundle = false;
}
}
}

async runTargetRequest(input: Entry) {
Expand Down Expand Up @@ -955,11 +997,50 @@ export class AssetGraphBuilder {

if (assets != null) {
for (let asset of assets) {
if (this.assetGraph.safeToIncrementallyBundle) {
let otherAsset = this.assetGraph.getNodeByContentKey(asset.id);
if (otherAsset != null) {
invariant(otherAsset.type === 'asset');
if (!this._areDependenciesEqualForAssets(asset, otherAsset.value)) {
this.assetGraph.safeToIncrementallyBundle = false;
}
} else {
// adding a new entry or dependency
this.assetGraph.safeToIncrementallyBundle = false;
}
}
this.changedAssets.set(asset.id, asset);
}
this.assetGraph.resolveAssetGroup(input, assets, request.id);
} else {
this.assetGraph.safeToIncrementallyBundle = false;
}
}

/**
* Used for incremental bundling of modified assets
*/
_areDependenciesEqualForAssets(asset: Asset, otherAsset: Asset): boolean {
let assetDependencies = Array.from(asset?.dependencies.keys()).sort();
let otherAssetDependencies = Array.from(
otherAsset?.dependencies.keys(),
).sort();

if (assetDependencies.length !== otherAssetDependencies.length) {
return false;
}

return assetDependencies.every((key, index) => {
if (key !== otherAssetDependencies[index]) {
return false;
}

return equalSet(
new Set(asset?.dependencies.get(key)?.symbols?.keys()),
new Set(otherAsset?.dependencies.get(key)?.symbols?.keys()),
);
});
}
}

function equalMap<K>(
Expand Down