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

Fix inline constants in shared bundles #9313

Merged
merged 12 commits into from
Oct 18, 2023
50 changes: 38 additions & 12 deletions packages/bundlers/default/src/DefaultBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,8 @@ function createIdealGraph(
// order the bundles are loaded.
let ancestorAssets = [];

let inlineConstantDeps = new DefaultMap(() => new Set());

for (let [bundleRootId, assetId] of bundleRootGraph.nodes.entries()) {
let reachable = new BitSet(assets.length);
reachableAssets.push(reachable);
Expand Down Expand Up @@ -994,6 +996,18 @@ function createIdealGraph(
let assetIndex = nullthrows(assetToIndex.get(node.value));
reachable.add(assetIndex);
reachableRoots[assetIndex].add(bundleRootId);

if (asset.meta.isConstantModule === true) {
let parents = assetGraph
.getIncomingDependencies(asset)
.map(dep => nullthrows(assetGraph.getAssetWithDependency(dep)));

for (let parent of parents) {
inlineConstantDeps.get(parent).add(asset);
}
}

return;
},
root,
{skipUnusedDependencies: true},
Expand Down Expand Up @@ -1143,6 +1157,16 @@ function createIdealGraph(
deleteBundle(bundleRoot);
}
}

function assignInlineConstants(parentAsset: Asset, bundle: Bundle) {
for (let inlineConstant of inlineConstantDeps.get(parentAsset)) {
if (!bundle.assets.has(inlineConstant)) {
bundle.assets.add(inlineConstant);
bundle.size += inlineConstant.stats.size;
}
}
}

// Step Insert Or Share: Place all assets into bundles or create shared bundles. Each asset
// is placed into a single bundle based on the bundle entries it is reachable from.
// This creates a maximally code split bundle graph with no duplication.
Expand All @@ -1153,19 +1177,15 @@ function createIdealGraph(
let asset = assets[i];
let manualSharedObject = manualAssetToConfig.get(asset);

if (asset.meta.isConstantModule === true) {
// Add assets to non-splittable bundles.
reachableRoots[i].forEach(nodeId => {
let assetId = bundleRootGraph.getNode(nodeId);
if (assetId == null) return; // deleted
let entry = assets[assetId];
let entryBundleId = nullthrows(bundleRoots.get(entry))[0];
let entryBundle = nullthrows(bundleGraph.getNode(entryBundleId));
invariant(entryBundle !== 'root');
entryBundle.assets.add(asset);
entryBundle.size += asset.stats.size;
});
if (bundleRoots.has(asset) && inlineConstantDeps.get(asset).size > 0) {
let entryBundleId = nullthrows(bundleRoots.get(asset))[0];
let entryBundle = nullthrows(bundleGraph.getNode(entryBundleId));
invariant(entryBundle !== 'root');
assignInlineConstants(asset, entryBundle);
}

if (asset.meta.isConstantModule === true) {
// Ignore constant modules as they are placed with their direct parents
continue;
}

Expand All @@ -1191,6 +1211,8 @@ function createIdealGraph(
invariant(entryBundle !== 'root');
entryBundle.assets.add(asset);
entryBundle.size += asset.stats.size;

assignInlineConstants(asset, entryBundle);
} else if (!ancestorAssets[nodeId]?.has(i)) {
// Filter out bundles from this asset's reachable array if
// bundle does not contain the asset in its ancestry
Expand Down Expand Up @@ -1365,6 +1387,8 @@ function createIdealGraph(
bundle.assets.add(asset);
bundle.size += asset.stats.size;

assignInlineConstants(asset, bundle);

for (let sourceBundleId of sourceBundles) {
if (bundleId !== sourceBundleId) {
bundleGraph.addEdge(sourceBundleId, bundleId);
Expand All @@ -1386,6 +1410,8 @@ function createIdealGraph(
invariant(bundle !== 'root');
bundle.assets.add(asset);
bundle.size += asset.stats.size;

assignInlineConstants(asset, bundle);
}
}
}
Expand Down
207 changes: 202 additions & 5 deletions packages/core/integration-tests/test/bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,203 @@ describe('bundler', function () {
]);
});

it('should support inline constants', async () => {
await fsFixture(overlayFS, __dirname)`
inline-constants-shared-bundles
one.html:
<script type="module" src="./one.js" />

two.html:
<script type="module" src="./two.js" />

one.js:
import {sharedFn} from './shared';
import {constant} from './constants';
sideEffectNoop('one' + sharedFn() + constant);

two.js:
import {sharedFn} from './shared';

sideEffectNoop('two' + sharedFn);

shared.js:
import {constant} from './constants.js';

export function sharedFn() {
return constant;
}

constants.js:
export const constant = 'constant';

package.json:
{
"@parcel/transformer-js": {
"unstable_inlineConstants": true
},
"@parcel/bundler-default": {
"minBundleSize": 0,
"minBundles": 3
}
}

yarn.lock:`;

let b = await bundle(
[
path.join(__dirname, 'inline-constants-shared-bundles', 'one.html'),
path.join(__dirname, 'inline-constants-shared-bundles', 'two.html'),
],
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: true,
sourceMaps: false,
shouldOptimize: false,
},
inputFS: overlayFS,
},
);

assertBundles(b, [
{
assets: ['one.html'],
},
{
assets: ['two.html'],
},
{
assets: ['one.js', 'shared.js', 'constants.js'],
},
{
assets: ['two.js', 'shared.js', 'constants.js'],
},
]);
});

it('should support inline constants with shared bundles', async () => {
await fsFixture(overlayFS, __dirname)`
inline-constants-shared-bundles
one.html:
<script type="module" src="./one.js" />

two.html:
<script type="module" src="./two.js" />

one.js:
import {sharedFn} from './shared';
import {constant} from './constants';
sideEffectNoop('one' + sharedFn() + constant);

two.js:
import {sharedFn} from './shared';

sideEffectNoop('two' + sharedFn);

shared.js:
import {constant} from './constants.js';

export function sharedFn() {
return constant;
}

constants.js:
export const constant = 'constant';

package.json:
{
"@parcel/transformer-js": {
"unstable_inlineConstants": true
},
"@parcel/bundler-default": {
"minBundleSize": 0
}
}

yarn.lock:`;

let b = await bundle(
[
path.join(__dirname, 'inline-constants-shared-bundles', 'one.html'),
path.join(__dirname, 'inline-constants-shared-bundles', 'two.html'),
],
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: true,
sourceMaps: false,
shouldOptimize: false,
},
inputFS: overlayFS,
},
);

assertBundles(b, [
{
assets: ['one.html'],
},
{
assets: ['two.html'],
},
{
assets: ['one.js', 'constants.js'],
},
{
assets: ['two.js'],
},
{
// shared bundle
assets: ['shared.js', 'constants.js'],
},
]);
});

it('should support inline constants in non-splittable bundles', async () => {
await fsFixture(overlayFS, __dirname)`
inline-constants-non-splittable
index.js:
import {sharedFn} from './shared';
sideEffectNoop(sharedFn());

shared.js:
import {constant} from './constants';

export function sharedFn() {
return constant;
}

constants.js:
export const constant = 'constant';

package.json:
{
"@parcel/transformer-js": {
"unstable_inlineConstants": true
}
}

yarn.lock:`;

let b = await bundle(
path.join(__dirname, 'inline-constants-non-splittable/index.js'),
{
mode: 'production',
defaultTargetOptions: {
shouldScopeHoist: true,
sourceMaps: false,
shouldOptimize: false,
},
inputFS: overlayFS,
},
);

assertBundles(b, [
{
assets: ['index.js', 'shared.js', 'constants.js'],
},
]);
});

describe('manual shared bundles', () => {
const dir = path.join(__dirname, 'manual-bundle');

Expand Down Expand Up @@ -1112,7 +1309,7 @@ describe('bundler', function () {
}]
}
}

.parcelrc:
{
"extends": "@parcel/config-default",
Expand All @@ -1133,14 +1330,14 @@ describe('bundler', function () {
return [asset];
}
});

index.html:
<script type="module">
import shared from './shared.js';
sideEffectNoop(shared);
sideEffectNoop(shared);
</script>
<script type="module" src="./index.js"></script>

index.js:
import shared from './shared.js';
sideEffectNoop(shared);
Expand Down Expand Up @@ -1388,7 +1585,7 @@ describe('bundler', function () {
],
},
{
assets: ['async.js', 'vendor-constants.js'],
assets: ['async.js'],
},
{
// Vendor MSB for JS
Expand Down
Loading