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: Add null check to removeStyleElement #245

Merged
merged 2 commits into from
May 30, 2017
Merged

fix: Add null check to removeStyleElement #245

merged 2 commits into from
May 30, 2017

Conversation

brandondurham
Copy link
Contributor

@brandondurham brandondurham commented May 30, 2017

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
Private function… no tests.

If relevant, did you update the README?
Not relevant.

Summary
style-loader was choking on the CSS Modules composes feature and throwing a fatal error. Adding a check to the removeStyleElement function in lib/addStyles.js seems to solve the problem.

Does this PR introduce a breaking change?
No.

Closes #182

@jsf-clabot
Copy link

jsf-clabot commented May 30, 2017

CLA assistant check
All committers have signed the CLA.

@michael-ciniawsky michael-ciniawsky changed the title fix: add check to removeStyleElement fix: add check to removeStyleElement May 30, 2017
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@michael-ciniawsky
Copy link
Member

@brandondurham Please sign the CLA, in case the bot bugs close and reopen the PR to trigger it again

@joshwiens joshwiens changed the title fix: add check to removeStyleElement fix: Adds null check to removeStyleElement May 30, 2017
lib/addStyles.js Outdated
style.parentNode.removeChild(style);

var idx = stylesInsertedAtTop.indexOf(style);

if(idx >= 0) {
if (idx >= 0) {
Copy link
Member

@alexander-akait alexander-akait May 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change not related lines to PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that. Subconscious flick of the wrist. Should be good to go.

@michael-ciniawsky michael-ciniawsky merged commit 0a4845c into webpack-contrib:master May 30, 2017
@michael-ciniawsky
Copy link
Member

@brandondurham Thx

@michael-ciniawsky michael-ciniawsky changed the title fix: Adds null check to removeStyleElement fix: Add null check to removeStyleElement May 30, 2017
@michael-ciniawsky michael-ciniawsky modified the milestone: 0.18.3 Jun 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS modules and composes colliding with style-loader?
7 participants