Sanitize proxy component name to minimize exceptions.

[leidegre]

Sanitize the display name to be able to act as a component name without causing issues if display name is not a valid identifier. This happens all the time with Redux components.

It's a very simple test that is more aggressive than the JavaScript identifier rules but it is also very simple.

Here's a Fiddle with some tests https://jsfiddle.net/rzyfr1hz/

J

[gaearon]

Nitpick: too many spaces before return.

[gaearon]

Thanks! Can you please add some tests? I think displayName tests are in consistency.js.

[leidegre]

I'll fix that together with the spaces later today.

[leidegre]

I took a look at the things you mentioned.

• The code is covered by existing tests (not every edge case).
• There's no way to tell/assert (in it's current form whether these exceptions were thrown or not)

I'd much rather like to provide the function as a utility and tests that in isolation. Just to ensure that its behavior is as it should. I'm not overly familiar with the source code but from what I can tell though, the component name is overshadowed anyway.

try {
  Object.defineProperty(ProxyComponent, 'name', {
    value: displayName
  });
} catch (err) { }

Unless I'm missing something. This is only really necessary if we can't create a dynamic constructor and defineProperty 'name' does not work which seems like the most spectacular edge case. At this point, do we really care that we didn't set a component name for the proxy? Wouldn't just be simpler to do this.

let displayName = getDisplayName(InitialComponent);
ProxyComponent = function () {
  return instantiate(() => CurrentComponent, this, arguments);
};
try {
  Object.defineProperty(ProxyComponent, 'name', {
    value: displayName
  });
} catch (err) {
  // ...
}

Please correct me if I'm off here.

[gaearon]

if we can't create a dynamic constructor

We have a test for this. Let’s just give that function a weird name.

[leidegre]

@gaearon like this #59 ?