preact-compat ^3.11.0 broke a link component

[bebraw]

Hi,

For some reason, a recent patch release broke Link at the webpack site. That led to a build failure. Fixing preact-compat version to 3.11.0 fixed the issue so it looks like there was some regression in a newer one.

I set up an example to study. Clone, yarn, npm run build, examine the build result (you should get a hefty error along TypeError: Cannot read property 'to' of undefined).

The generated code looks like this:

	var Link = function Link(_ref) {
	  var to = _ref.to,
	      props = _objectWithoutProperties(_ref, ['to']);

	  if ((0, _startsWith2.default)(to, 'http') || (0, _startsWith2.default)(to, '//')) {
	    return _react2.default.createElement('a', _extends({ href: to, target: '_blank' }, props));
	  }

	  if (false) {
	    return _react2.default.createElement(RRouter.Link, _extends({ to: to }, props));
	  }

	  return _react2.default.createElement('a', _extends({ href: to }, props));
	};

The error points at var to = _ref.to, meaning _ref is undefined. I guess the question is, what changed to cause this new behavior.

[developit]

Yikes! Looking into this. I'm wondering if it's something specific to Preact 6.x?

[developit]

@bebraw I'm seeing a bunch of react-dom in the stack for that error. Is that normal? Seems like React is doing the mounting.

[bebraw]

I think that's expected as it's build time error. react-dom generates the static version then.

Here's the component: https://github.com/webpack/webpack.js.org/blob/master/components/link/link.jsx .

Would it help if I reduced the issue to a particular commit of preact-compat?

[developit]

that'd be ideal, though I don't mind debugging. Just I am not sure I'm running this the right way (followed your steps but I don't see anything about Preact in the stack).

[bebraw]

Ok, I'll reduce it to a commit today then.

[developit]

@bebraw actually I'm thinking it's this:
50891a0#diff-1fdf421c05c1140f6d71444ea2b27638L571

If you think that looks suspicious (I do!) I will just revert it and publish as a 3.13.1. It didn't do anything for filesize and seems to wipe the constructor. Are you using PureComponent anywhere?

[bebraw]

There could be an indirect dep, but I don't recall the code should use it directly. That's a good commit to start checking from.

[developit]

(also a good one since I kinda want to revert that line anyway, it seems incorrect)

[bebraw]

I traced it down to 1fdf4fa. The previous commit, a01c0f1 works fine.

Btw, it would really help if you could set up a postinstall script to build the project if it's missing dist. Easier to consume from git. You'll end up with something like this.

[developit]

Hmm - haven't had anyone try to install from git for this repo before, usually people npm link. I can look into that, though I'll try to figure out the issue from that commit first.

[bebraw]

Yeah, installing from a git repo is a niche case.

[developit]

Happy to support it (though it'd be nice to do it all in the npm-script, maybe dropping node<4 would make that work). Just want to figure out the Link thing. Nothing so far :(

[developit]

Fix released as 3.13.1! 🎉

[bebraw]

Thanks. 👍

[KrofDrakula]

Just wanted to chime in and say that I just got bitten by a related bug to this and it fixed it. Saved my bacon. Thanks! 🍷

[developit]

Awesome!