Add overlay for build errors when using hmr

[lmontopo]

Fixes #413

Worked as a team with @sibartlett and @narsaynorath

[devongovett]

Siiiiick! Nice work 🎉

[lmontopo]

Any idea why these tests are failing? We've been trying to run the tests locally and different tests are failing. The one that's consistent is the hmr should log emitted errors test, but I'm confused as to why its raising a Uncaught TypeError: document.getElementById is not a function. If you have any ideas that'd be sweet! :)

[DeMoorJasper]

@lmontopo In the test environment some tests just fail from time to time.

To fix the Uncaught typeError however I think you have to change this part:

parcel/test/hmr.js

Lines 309 to 315 in 3ddaec9

	run(bundle, {
	console: {
	error(msg) {
	logs.push(msg);
	}
	}
	});

Into something like this:

run(bundle, {
      console: {
        error(msg) {
          logs.push(msg);
        }
      },
      document: {
        getElementById(id) {
          // return a mock element object
        },
        createElement(element) {
          // return a mock element object
        }
      }
    });

This is because the hmr tests are ran in node.js instead of a browser and we mock browsers global variables.

[lmontopo]

Thanks @DeMoorJasper, that makes sense (mocking those methods). We'll look into doing this soon (probably tomorrow) and then we'll push updates! :)

[devongovett]

what do you think about just using innerHTML for most of this? Might be easier to read?

[lmontopo]

just pushed up a new commit to try it this way! Took some time to figure out how to make it safe (i.e. how to ensure the message and stack trace are html encoded) but we figured it out. Let me know what you think.

[sibartlett]

We should use appendChild instead of append.

[lmontopo]

👍

[sibartlett]

Instead of wrapping the overlay in another div, we could try returningoverlay.firstChild instead:

function createErrorOverlay(data) {
  var overlay = document.createElement('div');

  overlay.innerHTML = (
    '<div id="' + OVERLAY_ID + ' ...' +
    '</div>'
  );

  return overlay.firstChild;
}

[lmontopo]

We decided to get rid of the html var as you suggested, but we kept the outer div because it just seemed more straightforward / easy to read and understand.

[sibartlett]

👍

[codecov-io]

Codecov Report

Merging #1074 into master will increase coverage by 4.36%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1074      +/-   ##
==========================================
+ Coverage   88.38%   92.75%   +4.36%     
==========================================
  Files          71       72       +1     
  Lines        3643     2815     -828     
==========================================
- Hits         3220     2611     -609     
+ Misses        423      204     -219

Impacted Files	Coverage Δ
src/assets/ReasonAsset.js	92.3% <0%> (-7.7%)	⬇️
src/visitors/matches-pattern.js	94.73% <0%> (-5.27%)	⬇️
src/assets/StylusAsset.js	84.78% <0%> (-2.28%)	⬇️
src/transforms/uglify.js	69.56% <0%> (-1.55%)	⬇️
src/WorkerFarm.js	91.93% <0%> (-1.51%)	⬇️
src/transforms/postcss.js	93.33% <0%> (-1.22%)	⬇️
src/transforms/babel.js	93.63% <0%> (-0.54%)	⬇️
src/utils/getTargetEngines.js	96.15% <0%> (-0.44%)	⬇️
src/assets/CoffeeScriptAsset.js	100% <0%> (ø)	⬆️
src/assets/TOMLAsset.js	100% <0%> (ø)	⬆️
... and 39 more

---

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 8a95f70...108223b. Read the comment docs.

[lmontopo]

yay! thanks for adding the tests @devongovett. Super psyched this was merged!

[sibartlett]

Yep, thanks for merging! I'm glad to have contributed to an awesome tool :)

[devongovett]

Thanks for contributing it! I'm excited to use it. :)