From 09984ab9ccdab6b29f89cd3af9bffef2b40c536c Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Mon, 30 Jan 2023 12:53:22 -0500 Subject: [PATCH] Don't blow up HMR when s don't have hrefs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Many browser extensions inject `` tags without `href` attributes, kinda like this one: ```html ``` When a page with a link to a stylesheet contains one of these `href`-less links, and the CSS on the page is updated, `updateLink()` tries to recreate _all_ `` tags, including the ones that don't link to anything. Consequently, `getAttribute('href')` returns `null`, which blows up string operations, specifically: ``` Uncaught TypeError: Cannot read properties of null (reading 'split') ``` This PR builds a safety check into the `updateLink()` function, so that the internal API doesn't change and downstream consumers don't have to think about it. ## 🚨 Test instructions I skimmed the tests and wasn't totally clear on how to write one for this (happy to do so if someone knowledgeable wants to point me in the right direction), but in lieu of that, here's a reliable way to reproduce the issue: ```bash mkdir hmr-test; \ cd hmr-test; \ curl https://gist.githubusercontent.com/nateabele/186825281fc96772468828786f86349a/raw/cb477118db4455be04df5abb2f3c1403467efc63/parcel-test.html > index.html; \ curl https://gist.githubusercontent.com/nateabele/186825281fc96772468828786f86349a/raw/b31f6421fd286a37431e346a60726e994946b133/parcel-test.css > index.css; \ parcel index.html ``` Then, make changes to `index.css` and save it. You should see the error in the console. --- packages/runtimes/hmr/src/loaders/hmr-runtime.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/runtimes/hmr/src/loaders/hmr-runtime.js b/packages/runtimes/hmr/src/loaders/hmr-runtime.js index cdfe7edaf5d..399219202d9 100644 --- a/packages/runtimes/hmr/src/loaders/hmr-runtime.js +++ b/packages/runtimes/hmr/src/loaders/hmr-runtime.js @@ -303,6 +303,11 @@ function getParents(bundle, id) /*: Array<[ParcelRequire, string]> */ { } function updateLink(link) { + var href = link.getAttribute('href'); + + if (!href) { + return; + } var newLink = link.cloneNode(); newLink.onload = function () { if (link.parentNode !== null) { @@ -313,7 +318,7 @@ function updateLink(link) { newLink.setAttribute( 'href', // $FlowFixMe - link.getAttribute('href').split('?')[0] + '?' + Date.now(), + href.split('?')[0] + '?' + Date.now(), ); // $FlowFixMe link.parentNode.insertBefore(newLink, link.nextSibling);