Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix: add a not null check for the content property before throwing error #404

Merged
merged 3 commits into from
Apr 24, 2017

Conversation

sresant
Copy link
Contributor

@sresant sresant commented Feb 13, 2017

This check will fix #42 for webpack2 users.

@codecov
Copy link

codecov bot commented Feb 13, 2017

Codecov Report

Merging #404 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
- Coverage   90.42%   90.38%   -0.04%     
==========================================
  Files           6        6              
  Lines         355      364       +9     
  Branches       74       77       +3     
==========================================
+ Hits          321      329       +8     
- Misses         34       35       +1
Impacted Files Coverage Δ
index.js 91.03% <100%> (+0.2%) ⬆️
ExtractedModule.js 95.12% <0%> (-2.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e831507...12a6e44. Read the comment docs.

@bebraw
Copy link
Contributor

bebraw commented Feb 17, 2017

Would it be possible to add a test against this case so we don't break it in the future? Good apart from that.

@sresant
Copy link
Contributor Author

sresant commented Feb 27, 2017

@bebraw Yea sure, it has been a busy couple weeks...I'll try getting some test this week.

@sresant
Copy link
Contributor Author

sresant commented Feb 28, 2017

@bebraw After looking at this a little further, any suggestions on testing this with watch mode running? I looked at the current set of test suites and it looks like they just run a single compilation using webpack. This test would require running watching mode.

@bebraw
Copy link
Contributor

bebraw commented Feb 28, 2017

@sresant That's a good point. I would look into webpack core to see how it handles watch mode related tests.

If there's no way, let's merge. 👍

@sresant
Copy link
Contributor Author

sresant commented Mar 2, 2017

@bebraw Looking at it the webpack core version of the watchmode test suite is different from what this test would require to work so might be more difficult to implement :(

@bebraw
Copy link
Contributor

bebraw commented Mar 2, 2017

@d3viant0ne Any thoughts on this? Small change.

@michael-ciniawsky michael-ciniawsky changed the title Add a not null check for the content property before throwing error fix: add a not null check for the content property before throwing error Apr 22, 2017
@michael-ciniawsky michael-ciniawsky added this to the 2.1.x milestone Apr 22, 2017
@bebraw
Copy link
Contributor

bebraw commented Apr 24, 2017

@michael-ciniawsky Maybe merge? I don't see this breaking anything so might as well. A comment would be good, though.

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.

@sresant Please add a comment in case this causes regressions

@michael-ciniawsky
Copy link
Member

@bebraw Yep it should be fine, but I'm not 💯 with ETWP yet tbh 😛

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.

@sresant Thx

@michael-ciniawsky
Copy link
Member

@sresant Please close and reopen the PR to trigger the CLA Bot again and sign the CLA :)

@sresant sresant closed this Apr 24, 2017
@sresant sresant reopened this Apr 24, 2017
@michael-ciniawsky michael-ciniawsky merged commit 58dd5d3 into webpack-contrib:master Apr 24, 2017
@nitruxa
Copy link

nitruxa commented May 11, 2017

Took this change - build goes green, but in console every second build this error occurs:

Uncaught TypeError: Cannot read property 'call' of undefined
    at __webpack_require__ (bootstrap 278d682…:52)
    at Object.1145 (styles.css?7f97:4)
    at __webpack_require__ (bootstrap 278d682…:52)
    at Object.webpackJsonp.1098.module.exports.Object.defineProperty.value (bundle.js:1724)
    at __webpack_require__ (bundle.js:21)
    at Object.<anonymous> (bundle.js:130)
    at __webpack_require__ (bundle.js:21)
    at Object.webpackJsonp.1098.module.exports.Object.defineProperty.value (bundle.js:55)
    at __webpack_require__ (bundle.js:21)
    at webpackJsonp.1098.module.exports.Object.defineProperty.value (bundle.js:41)

@michael-ciniawsky michael-ciniawsky modified the milestone: 2.1.2 Jun 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected: "ERROR in <blah> doesn't export content"
4 participants