Repository: parcel-bundler/parcel (js)

Issue:
Referencing a css file from js and html builds incorrect html script references
<!---
Thanks for filing an issue 😄 ! Before you submit, please read the following:

Search open/closed issues before submitting since someone might have asked the same thing before!
-->

# 🐛 bug report

<!--- Provide a general summary of the issue here -->

When you build an app that has multiple entrypoints which both point to the same JS and CSS file (this is a minimal repro use-case) and also have that JS file point to the same CSS file the html bundle that gets built returns the css bundle path in 2 `link` tags, the one that was already present and a new one inserted as it's also referenced by the JS. Besides this is also puts that css bundle path in the script tag, where the JS bundle should be referenced.

## 🤔 Expected Behavior

<!--- Tell us what should happen -->

The script tag should reference the JS bundle and ideally but optionally the CSS bundle should only be referenced once. So basically not add an additional link tag if the css file is already part of the html bundle.

## 😯 Current Behavior

JS does not get loaded and CSS bundle is referenced 3 times.

## 💁 Possible Solution

<!--- Not obligatory, but suggest a fix/reason for the bug -->

I solved this by just removing the css reference in the JS file, which is a good solution for this specific use-case as it should really only be referenced once.

But this seems like a bug that could potentially have a larger impact on more useful use-cases.

Tried to debug it as far as I could and got stuck at `replaceURLReferences` where `bundleGraph.getBundlesInBundleGroup(resolved.value)` returned 3 bundles with the first being the JS which I wanted to reference in the script tag.

And the css bundle which is both bundle 1 and 2 in the link tag. Maybe this is a bundler bug as these are all the same `bundle-group`? and it should probably be 2 groups, one for the CSS and one for the JS. I wish I knew more about the graph and bundling logic of Parcel but it's currently a bit of a black box so I should probably focus my efforts towards the parts I know a lot about,

## 💻 Code Sample

Reproducible example: (also made a branch with a test-case: https://github.com/parcel-bundler/parcel/tree/fix-html-bug)

`parcel build ./*.html`

`a.html`
```html
<head>
	<link rel="stylesheet" href="./style.css">
</head>

<body>
	<script src="./index.js"></script>
</body>
```

`b.html`
```html
<head>
	<link rel="stylesheet" href="./style.css">
</head>

<body>
	<script src="./index.js"></script>
</body>
```

`index.js`
```js
import "./style.css";
```

`index.css`
```css
html, body {}
```

## 🌍 Your Environment

| Software         | Version(s) |
| ---------------- | ---------- |
| Parcel           | v2.0.0-beta.1


Pull request description:
Fix html referencing wrong bundle
<!---
Thanks for filing a pull request 😄 ! Before you submit, please read the following:

Search open/closed issues before submitting since someone might have pushed the same thing before!
-->

# ↪️ Pull Request

<!---
Provide a general summary of the pull request here
Please look for any issues that this PR resolves and tag them in the PR.
-->

This PR fixes a bug in `replaceURLReferences` where it sometimes returns a reference to an incorrect bundle as it does not actually use the entryBundle.

Thanks @mischnic for mentioning bundle_group has an entryAssetId

See #4772 for context

Fixes #4772


Patch:
diff --git a/packages/core/integration-tests/test/integration/html-multi-entry/style.css b/packages/core/integration-tests/test/integration/html-multi-entry/style.css
new file mode 100644
index 000000000..e69de29bb
diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js
index f01ef500b..06c3bf2e7 100644
--- a/packages/core/utils/src/replaceBundleReferences.js
+++ b/packages/core/utils/src/replaceBundleReferences.js
@@ -64,8 +64,13 @@ export function replaceURLReferences({
     }
 
     invariant(resolved.type === 'bundle_group');
-    let entryBundle = bundleGraph.getBundlesInBundleGroup(resolved.value).pop();
-    if (entryBundle.isInline) {
+    let entryAssetId = resolved.value.entryAssetId;
+    let entryBundle = bundleGraph
+      .getBundlesInBundleGroup(resolved.value)
+      .find(b => {
+        return !!b.getEntryAssets().find(a => a.id === entryAssetId);
+      });
+    if (!entryBundle || entryBundle.isInline) {
       // If a bundle is inline, it should be replaced with inline contents,
       // not a URL.
       continue;


Test patch:
diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js
index 30ccfe615..d938b3fb7 100644
--- a/packages/core/integration-tests/test/html.js
+++ b/packages/core/integration-tests/test/html.js
@@ -1370,6 +1370,73 @@ describe('html', function() {
     checkHtml('g.html');
   });
 
+  it('should include the correct paths when using multiple entries and referencing style from html and js', async function() {
+    let b = await bundle(
+      path.join(__dirname, '/integration/html-multi-entry/*.html'),
+      {
+        production: true,
+        scopeHoist: true,
+      },
+    );
+
+    await assertBundles(b, [
+      {
+        name: 'a.html',
+        type: 'html',
+        assets: ['a.html'],
+      },
+      {
+        name: 'b.html',
+        type: 'html',
+        assets: ['b.html'],
+      },
+      {
+        type: 'css',
+        assets: ['style.css'],
+      },
+      {
+        type: 'js',
+        assets: ['index.js'],
+      },
+    ]);
+
+    let firstHtmlFile = await outputFS.readFile(
+      path.join(distDir, 'a.html'),
+      'utf8',
+    );
+
+    let secondHtmlFile = await outputFS.readFile(
+      path.join(distDir, 'b.html'),
+      'utf8',
+    );
+
+    let bundles = b.getBundles();
+    let cssBundle = path.basename(
+      bundles.find(bundle => bundle.filePath.endsWith('.css')).filePath,
+    );
+    let jsBundle = path.basename(
+      bundles.find(bundle => bundle.filePath.endsWith('.js')).filePath,
+    );
+
+    assert(
+      firstHtmlFile.includes(cssBundle),
+      `a.html should include a reference to ${cssBundle}`,
+    );
+    assert(
+      secondHtmlFile.includes(cssBundle),
+      `b.html should include a reference to ${cssBundle}`,
+    );
+
+    assert(
+      firstHtmlFile.includes(jsBundle),
+      `a.html should include a reference to ${jsBundle}`,
+    );
+    assert(
+      secondHtmlFile.includes(jsBundle),
+      `b.html should include a reference to ${jsBundle}`,
+    );
+  });
+
   it('should invalidate parent bundle when inline bundles change', async function() {
     // copy into memory fs
     await ncp(
diff --git a/packages/core/integration-tests/test/integration/html-multi-entry/a.html b/packages/core/integration-tests/test/integration/html-multi-entry/a.html
new file mode 100644
index 000000000..3adf1b064
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/html-multi-entry/a.html
@@ -0,0 +1,7 @@
+<head>
+	<link rel="stylesheet" href="./style.css">
+</head>
+
+<body>
+	<script src="./index.js"></script>
+</body>
\ No newline at end of file
diff --git a/packages/core/integration-tests/test/integration/html-multi-entry/b.html b/packages/core/integration-tests/test/integration/html-multi-entry/b.html
new file mode 100644
index 000000000..3adf1b064
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/html-multi-entry/b.html
@@ -0,0 +1,7 @@
+<head>
+	<link rel="stylesheet" href="./style.css">
+</head>
+
+<body>
+	<script src="./index.js"></script>
+</body>
\ No newline at end of file
diff --git a/packages/core/integration-tests/test/integration/html-multi-entry/index.js b/packages/core/integration-tests/test/integration/html-multi-entry/index.js
new file mode 100644
index 000000000..fa916f131
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/html-multi-entry/index.js
@@ -0,0 +1,1 @@
+import "./style.css";
\ No newline at end of file