Skip to content

Commit

Permalink
Address bug by updating an asset reference and merge conditions (#8762)
Browse files Browse the repository at this point in the history
  • Loading branch information
AGawrys committed Jan 17, 2023
1 parent ddae31a commit 7023c08
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 10 deletions.
47 changes: 37 additions & 10 deletions packages/bundlers/default/src/DefaultBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,6 @@ function createIdealGraph(
}
},
});

// Step Merge Type Change Bundles: Clean up type change bundles within the exact same bundlegroups
for (let [nodeIdA, a] of bundleGraph.nodes) {
//if bundle b bundlegroups ==== bundle a bundlegroups then combine type changes
Expand Down Expand Up @@ -694,6 +693,11 @@ function createIdealGraph(
//asset node type
let asset = node.value;
if (asset.bundleBehavior != null || root.type !== asset.type) {
if (root.type !== asset.type && !bundleRoots.has(asset)) {
// A type may not necessarily be a bundleRoot since we've merged at this point
// So we must add that asset in as an island at the very least
reachableRoots.addNodeByContentKeyIfNeeded(node.value.id, node.value);
}
actions.skipChildren();
return;
}
Expand Down Expand Up @@ -873,8 +877,9 @@ function createIdealGraph(
if (
entries.has(a) ||
!a.isBundleSplittable ||
getBundleFromBundleRoot(a).needsStableName ||
getBundleFromBundleRoot(a).bundleBehavior === 'isolated'
(bundleRoots.get(a) &&
(getBundleFromBundleRoot(a).needsStableName ||
getBundleFromBundleRoot(a).bundleBehavior === 'isolated'))
) {
reachableEntries.push(a);
} else {
Expand Down Expand Up @@ -1115,12 +1120,6 @@ function createIdealGraph(
bundleGraph.removeNode(nullthrows(bundles.get(bundleRoot.id)));
bundleRoots.delete(bundleRoot);
bundles.delete(bundleRoot.id);
if (reachableRoots.hasContentKey(bundleRoot.id)) {
reachableRoots.replaceNodeIdsConnectedTo(
reachableRoots.getNodeIdByContentKey(bundleRoot.id),
[],
);
}
if (bundleRootGraph.hasContentKey(bundleRoot.id)) {
bundleRootGraph.removeNode(
bundleRootGraph.getNodeIdByContentKey(bundleRoot.id),
Expand Down Expand Up @@ -1155,6 +1154,17 @@ function createIdealGraph(
invariant(a !== 'root' && b !== 'root');
let bundleRootB = nullthrows(b.mainEntryAsset);
let mainBundleRoot = nullthrows(a.mainEntryAsset);
let bundleGroupOfMain = nullthrows(bundleRoots.get(mainBundleRoot))[1];
// If our merging bundle is already a combination of bundles, all previous root assets must be updated as well
for (let movingAsset of b.assets) {
if (movingAsset === bundleRootB) continue;
if (bundleRoots.has(movingAsset)) {
bundleRoots.set(movingAsset, [mainNodeId, bundleGroupOfMain]);
bundles.set(movingAsset.id, mainNodeId);
}
replaceAssetReference(movingAsset, b, a);
}

for (let asset of b.assets) {
a.assets.add(asset);
a.size += asset.stats.size;
Expand All @@ -1176,10 +1186,13 @@ function createIdealGraph(
for (let nodeId of bundleGraph.getNodeIdsConnectedTo(otherNodeId)) {
bundleGraph.addEdge(nodeId, mainNodeId);
}
replaceAssetReference(bundleRootB, b, a);
deleteBundle(bundleRootB);
let bundleGroupOfMain = nullthrows(bundleRoots.get(mainBundleRoot))[1];
bundleRoots.set(bundleRootB, [mainNodeId, bundleGroupOfMain]);
bundles.set(bundleRootB.id, mainNodeId);

bundleRoots.delete(bundleRootB);
bundles.delete(bundleRootB.id);
}
function getBundleFromBundleRoot(bundleRoot: BundleRoot): Bundle {
let bundle = bundleGraph.getNode(
Expand All @@ -1188,6 +1201,20 @@ function createIdealGraph(
invariant(bundle !== 'root' && bundle != null);
return bundle;
}
function replaceAssetReference(
bundleRoot: BundleRoot,
toReplace: Bundle,
replaceWith: Bundle,
): void {
let replaceAssetReference = assetReference.get(bundleRoot).map(entry => {
let bundle = entry[1];
if (bundle == toReplace) {
return [entry[0], replaceWith];
}
return entry;
});
assetReference.set(bundleRoot, replaceAssetReference);
}

return {
bundleGraph,
Expand Down
18 changes: 18 additions & 0 deletions packages/core/integration-tests/test/css-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -674,4 +674,22 @@ describe('css modules', () => {
let res = await run(b);
assert.deepEqual(res, ['_4fY2uG_foo', '--wGsoEa_from-js']);
});

it('should group together css and css modules into one bundle', async function () {
let b = await bundle(
path.join(__dirname, '/integration/css-module-css-siblings/index.html'),
);

let res = [];
await runBundle(
b,
b.getBundles().find(b => b.name === 'index.html'),
{
sideEffect: s => res.push(s),
},
);
assert.deepEqual(res, [
['mainJs', '_1ZEqVW_myClass', 'j1UkRG_myOtherClass'],
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.myClass {
color: blue;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.myOtherClass {
color: red;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
font-size: 10px;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<html>
<head>
<script type="module" src="./main.js"></script>
<link rel="stylesheet" href="./global.css" />
</head>
<body></body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Adopted from https://github.com/nartallax/parcel-css-bug
// To address https://github.com/parcel-bundler/parcel/issues/8716
import * as aCss from "./a.module.css"
import * as bCss from "./b.module.css"

sideEffect(['mainJs', aCss.myClass, bCss.myOtherClass]);
Empty file.

0 comments on commit 7023c08

Please sign in to comment.