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

A draft conditional bundling approach #9661

Draft
wants to merge 14 commits into
base: v2
Choose a base branch
from
1 change: 1 addition & 0 deletions packages/bundlers/default/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
},
"dependencies": {
"@parcel/diagnostic": "2.12.0",
"@parcel/feature-flags": "2.12.0",
"@parcel/graph": "3.2.0",
"@parcel/plugin": "2.12.0",
"@parcel/rust": "2.12.0",
Expand Down
8 changes: 7 additions & 1 deletion packages/bundlers/default/src/DefaultBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {validateSchema, DefaultMap, globToRegex} from '@parcel/utils';
import nullthrows from 'nullthrows';
import path from 'path';
import {encodeJSONKeyComponent} from '@parcel/diagnostic';
import {getFeatureFlag} from '@parcel/feature-flags';

type Glob = string;

Expand Down Expand Up @@ -95,6 +96,7 @@ const dependencyPriorityEdges = {
sync: 1,
parallel: 2,
lazy: 3,
conditional: 4,
};

type DependencyBundleGraph = ContentGraph<
Expand Down Expand Up @@ -495,7 +497,9 @@ function createIdealGraph(

if (
node.type === 'dependency' &&
node.value.priority === 'lazy' &&
(node.value.priority === 'lazy' ||
(getFeatureFlag('conditionalBundling') &&
node.value.priority === 'conditional')) &&
parentAsset
) {
// Don't walk past the bundle group assets
Expand Down Expand Up @@ -584,6 +588,8 @@ function createIdealGraph(
}
if (
dependency.priority === 'lazy' ||
(getFeatureFlag('conditionalBundling') &&
dependency.priority === 'conditional') ||
childAsset.bundleBehavior === 'isolated' // An isolated Dependency, or Bundle must contain all assets it needs to load.
) {
if (bundleId == null) {
Expand Down
68 changes: 67 additions & 1 deletion packages/core/core/src/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
Environment,
InternalSourceLocation,
Target,
Condition,
} from './types';
import type AssetGraph from './AssetGraph';
import type {ProjectPath} from './projectPath';
Expand All @@ -42,6 +43,7 @@ import {getBundleGroupId, getPublicId} from './utils';
import {ISOLATED_ENVS} from './public/Environment';
import {fromProjectPath, fromProjectPathRelative} from './projectPath';
import {HASH_REF_PREFIX} from './constants';
import {getFeatureFlag} from '@parcel/feature-flags';

export const bundleGraphEdgeTypes = {
// A lack of an edge type indicates to follow the edge while traversing
Expand Down Expand Up @@ -87,6 +89,7 @@ type BundleGraphOpts = {|
bundleContentHashes: Map<string, string>,
assetPublicIds: Set<string>,
publicIdByAssetId: Map<string, string>,
conditions: Map<string, Condition>,
|};

type SerializedBundleGraph = {|
Expand All @@ -95,6 +98,7 @@ type SerializedBundleGraph = {|
bundleContentHashes: Map<string, string>,
assetPublicIds: Set<string>,
publicIdByAssetId: Map<string, string>,
conditions: Map<string, Condition>,
|};

function makeReadOnlySet<T>(set: Set<T>): $ReadOnlySet<T> {
Expand Down Expand Up @@ -135,22 +139,26 @@ export default class BundleGraph {
/** The internal core Graph structure */
_graph: ContentGraph<BundleGraphNode, BundleGraphEdgeType>;
_bundlePublicIds /*: Set<string> */ = new Set<string>();
_conditions /*: Set<Condition> */ = new Set<Condition>();

constructor({
graph,
publicIdByAssetId,
assetPublicIds,
bundleContentHashes,
conditions,
}: {|
graph: ContentGraph<BundleGraphNode, BundleGraphEdgeType>,
publicIdByAssetId: Map<string, string>,
assetPublicIds: Set<string>,
bundleContentHashes: Map<string, string>,
conditions: Set<Condition>,
|}) {
this._graph = graph;
this._assetPublicIds = assetPublicIds;
this._publicIdByAssetId = publicIdByAssetId;
this._bundleContentHashes = bundleContentHashes;
this._conditions = conditions;
}

/**
Expand All @@ -167,6 +175,9 @@ export default class BundleGraph {
let assetGroupIds = new Map();
let dependencies = new Map();
let assetGraphNodeIdToBundleGraphNodeId = new Map<NodeId, NodeId>();
let conditions = new Map();

let placeholderToDependency = new Map();

let assetGraphRootNode =
assetGraph.rootNodeId != null
Expand All @@ -189,6 +200,18 @@ export default class BundleGraph {
}
} else if (node != null && node.type === 'asset_group') {
assetGroupIds.set(nodeId, assetGraph.getNodeIdsConnectedFrom(nodeId));
} else if (
getFeatureFlag('conditionalBundling') &&
node != null &&
node.type === 'dependency'
) {
// The dependency placeholders in the `importCond` calls that will be in the transformed
// code need to be mapped to the "real" depenencies, so we need access to a map of placeholders
// to dependencies
const dep = node.value;
if (dep.meta?.placeholder != null) {
placeholderToDependency.set(dep.meta.placeholder, dep);
}
}
}

Expand All @@ -198,6 +221,44 @@ export default class BundleGraph {
walkVisited.add(nodeId);

let node = nullthrows(assetGraph.getNode(nodeId));

if (getFeatureFlag('conditionalBundling') && node.type === 'asset') {
const asset = node.value;
if (Array.isArray(asset.meta.conditions)) {
for (const condition of asset.meta.conditions ?? []) {
// Resolve the placeholders that were attached to the asset in JSTransformer to dependencies,
// as well as create a public id for the condition.

// $FlowFixMe[incompatible-type]
const {
key,
ifTruePlaceholder,
ifFalsePlaceholder,
}: {
key: string,
ifTruePlaceholder: string,
ifFalsePlaceholder: string,
...
} = condition;

const condHash = hashString(
`${key}:${ifTruePlaceholder}:${ifFalsePlaceholder}`,
);
const condPublicId = getPublicId(condHash, v => conditions.has(v));

conditions.set(condition, {
publicId: condPublicId,
// FIXME support the same condition used across multiple assets..
assets: new Set([asset]),
key,
ifTrueDependency: placeholderToDependency.get(ifTruePlaceholder),
ifFalseDependency:
placeholderToDependency.get(ifFalsePlaceholder),
});
}
}
}

if (
node.type === 'dependency' &&
node.value.symbols != null &&
Expand Down Expand Up @@ -434,11 +495,13 @@ export default class BundleGraph {
);
}
}

return new BundleGraph({
graph,
assetPublicIds,
bundleContentHashes: new Map(),
publicIdByAssetId,
conditions,
});
}

Expand All @@ -449,6 +512,7 @@ export default class BundleGraph {
assetPublicIds: this._assetPublicIds,
bundleContentHashes: this._bundleContentHashes,
publicIdByAssetId: this._publicIdByAssetId,
conditions: this._conditions,
};
}

Expand All @@ -458,6 +522,7 @@ export default class BundleGraph {
assetPublicIds: serialized.assetPublicIds,
bundleContentHashes: serialized.bundleContentHashes,
publicIdByAssetId: serialized.publicIdByAssetId,
conditions: serialized.conditions,
});
}

Expand Down Expand Up @@ -1209,7 +1274,8 @@ export default class BundleGraph {
.some(
node =>
node?.type === 'dependency' &&
node.value.priority === Priority.lazy &&
(node.value.priority === Priority.lazy ||
node.value.priority === Priority.conditional) &&
node.value.specifierType !== SpecifierType.url,
)
) {
Expand Down
5 changes: 4 additions & 1 deletion packages/core/core/src/SymbolPropagation.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ export function propagateSymbols({
namespaceReexportedSymbols.add('*');
} else {
for (let incomingDep of incomingDeps) {
if (incomingDep.value.symbols == null) {
if (
incomingDep.value.symbols == null ||
incomingDep.value.priority === 3
) {
if (incomingDep.value.sourceAssetId == null) {
// The root dependency on non-library builds
isEntry = true;
Expand Down
88 changes: 88 additions & 0 deletions packages/core/core/src/public/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import {targetToInternalTarget} from './Target';
import {fromInternalSourceLocation} from '../utils';
import BundleGroup, {bundleGroupToInternalBundleGroup} from './BundleGroup';
import {getFeatureFlag} from '@parcel/feature-flags';

Check failure on line 33 in packages/core/core/src/public/BundleGraph.js

View workflow job for this annotation

GitHub Actions / Lint

'getFeatureFlag' is defined but never used

// Friendly access for other modules within this package that need access
// to the internal bundle.
Expand Down Expand Up @@ -326,4 +327,91 @@
targetToInternalTarget(target),
);
}

// Given a set of dependencies, return any conditions where those dependencies are either
// the true or false dependency for those conditions. This is currently used to work out which
// conditions belong to a bundle in packaging.
getConditionsForDependencies(deps: Array<Dependency>): Set<{|
key: string,
dependency: Dependency,
ifTrue: string,
ifFalse: string,
|}> {
// FIXME improve these lookups
const conditions = new Set();
const depIds = deps.map(dep => dep.id);
for (const condition of this.#graph._conditions.values()) {
if (
depIds.includes(condition.ifTrueDependency.id) ||
depIds.includes(condition.ifFalseDependency.id)
) {
const [trueAsset, falseAsset] = [
condition.ifTrueDependency,
condition.ifFalseDependency,
].map(dep => {
const resolved = nullthrows(this.#graph.resolveAsyncDependency(dep));
if (resolved.type === 'asset') {
return resolved.value;
} else {
return this.#graph.getAssetById(resolved.value.entryAssetId);
}
});
conditions.add({
key: condition.key,
dependency: deps.find(
dep => dep.id === condition.ifTrueDependency.id,
),
ifTrue: this.#graph.getAssetPublicId(trueAsset),
ifFalse: this.#graph.getAssetPublicId(falseAsset),
});
}
}
// FIXME work out what's missing here.. (Flow)
return conditions;
}

// This is used to generate information for building a manifest that can
// be used by a webserver to understand which conditions are used by which bundles,
// and which bundles those conditions require depending on what they evaluate to.
unstable_getConditionalBundleMapping(): {|
[string]: {|
bundlesWithCondition: Array<TBundle>,
ifTrueBundles: Array<TBundle>,
ifFalseBundles: Array<TBundle>,
|},
|} {
let conditions = {};
// Convert the internal references in conditions to public API references
for (const cond of this.#graph._conditions.values()) {
let assets = Array.from(cond.assets).map(asset =>
nullthrows(this.getAssetById(asset.id)),
);
let bundles = new Set();
let ifTrueBundles = [];
let ifFalseBundles = [];
for (const asset of assets) {
const bundlesWithAsset = this.getBundlesWithAsset(asset);
for (const bundle of bundlesWithAsset) {
bundles.add(bundle);
}
const assetDeps = this.getDependencies(asset);
const depToBundles = dep => {
const publicDep = nullthrows(
assetDeps.find(assetDep => dep.id === assetDep.id),
);
const resolved = nullthrows(this.resolveAsyncDependency(publicDep));
invariant(resolved.type === 'bundle_group');
return this.getBundlesInBundleGroup(resolved.value);
};
ifTrueBundles.push(...depToBundles(cond.ifTrueDependency));
ifFalseBundles.push(...depToBundles(cond.ifFalseDependency));
}
conditions[cond.key] = {
bundlesWithCondition: Array.from(bundles),
ifTrueBundles,
ifFalseBundles,
};
}
return conditions;
}
}
1 change: 1 addition & 0 deletions packages/core/core/src/public/MutableBundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
ParcelOptions,
BundleGroup as InternalBundleGroup,
BundleNode,
Condition,

Check failure on line 16 in packages/core/core/src/public/MutableBundleGraph.js

View workflow job for this annotation

GitHub Actions / Lint

'Condition' is defined but never used
} from '../types';

import invariant from 'assert';
Expand Down
10 changes: 10 additions & 0 deletions packages/core/core/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export const Priority = {
sync: 0,
parallel: 1,
lazy: 2,
conditional: 3,
};

// Must match package_json.rs in node-resolver-rs.
Expand Down Expand Up @@ -540,6 +541,7 @@ export type Bundle = {|
displayName: ?string,
pipeline: ?string,
manualSharedBundle?: ?string,
conditions?: Map<string, string>,
|};

export type BundleNode = {|
Expand Down Expand Up @@ -578,3 +580,11 @@ export type ValidationOpts = {|
|};

export type ReportFn = (event: ReporterEvent) => void | Promise<void>;

export type Condition = {|
publicId: string,
assets: Set<Asset>,
key: string,
ifTrueDependency: Dependency,
ifFalseDependency: Dependency,
|};
1 change: 1 addition & 0 deletions packages/core/feature-flags/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type FeatureFlags = _FeatureFlags;
export const DEFAULT_FEATURE_FLAGS: FeatureFlags = {
exampleFeature: false,
configKeyInvalidation: false,
conditionalBundling: false,
};

let featureFlagValues: FeatureFlags = {...DEFAULT_FEATURE_FLAGS};
Expand Down
6 changes: 6 additions & 0 deletions packages/core/feature-flags/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ export type FeatureFlags = {|
* `config.getConfigFrom(..., {packageKey: '...'})` and the value itself hasn't changed.
*/
+configKeyInvalidation: boolean,
/**
* Enables experimental "conditional bundling" - this allows the use of `importCond` syntax
* in order to have (consumer) feature flag driven bundling. This feature is very experimental,
* and requires server-side support.
*/
+conditionalBundling: boolean,
|};
Loading
Loading