Don't blow up HMR when <link />s don't have hrefs #8800
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Many browser extensions inject `<link />` tags without `href` attributes, kinda like this one:
```html
<link type="text/css" rel="stylesheet" id="dark-mode-custom-link" />
```
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_ `<link />` 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.
marcins
pushed a commit
to marcins/parcel
that referenced
this pull request
Jul 14, 2023
* upstream/v2: Sort global deps before injecting imports (parcel-bundler#8818) Sort CSS module exports (parcel-bundler#8817) fix: add extra information to unique bundles (parcel-bundler#8784) Don't blow up HMR when <link />s don't have hrefs (parcel-bundler#8800)
lettertwo
added a commit
that referenced
this pull request
Nov 6, 2023
* upstream/v2: (128 commits) [webextension] Add support for `chrome_style` (#8867) Switch to SWC minifier by default (#8860) Use BitSet for bundler intersections (#8862) best key logic truncating package names (#8865) Add support for loadConfig to resolver plugins (#8847) Missing edge for multiple targets (#8854) Split large runtime manifest into separate bundles (#8837) Improvements to new resolver (#8844) Fix published files for resolver New resolver implementation in Rust (#8807) Update yarn.lock (#8843) Bump napi-rs to latest (#8838) Support .proxyrc.cjs (#8833) Sort global deps before injecting imports (#8818) Sort CSS module exports (#8817) fix: add extra information to unique bundles (#8784) Don't blow up HMR when <link />s don't have hrefs (#8800) v2.8.3 Changelog for v2.8.3 Address bug by updating an asset reference and merge conditions (#8762) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Many browser extensions inject
<link />tags withouthrefattributes, kinda like this one: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<link />tags, including the ones that don't link to anything. Consequently,getAttribute('href')returnsnull, which blows up string operations, specifically: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:
Then, make changes to
index.cssand save it. You should see the error in the console.