fix: Ignore invalid exported components

[fwouts]

Playroom currently expects that every single export in the components module is a valid React component.

For example, export const NotAComponent = 1 would result in Playroom exposing a broken component called <NotAComponent>.

Another issue pointed in #138 is that an exported value of undefined will crash Playroom. This happens in particular when a TypeScript type is explicitly exported in an export declaration statement such as export { Component, SomeType }.

This can all be resolved by filtering out any value that isn't a function. This isn't perfect, as it will still accept non-React functions and classes, but should at least fix a good portion of bugs experienced by people trying out the tool for the first time.

This fixes #138.

[fwouts]

I'm not quite sure what's going on with the broken test, would welcome some help debugging it.

[fwouts]

Woohoo, it's passing! Ready for review 😃

[fwouts]

@michaeltaranto @markdalgleish What's the current status of the Playroom project? Has it been put on hold because of COVID-19?

[michaeltaranto]

Sorry mate, the world did flip upside down, but that is not why this PR has not been merged reviewed — we have just been busy elsewhere in our ecosystem. Looking at a few tweaks to playroom at the moment, so will check this out soon.

Thanks for taking the time.

[fwouts]

Tests keep failing, I'm not sure what's going on. The Cypress code uses patterns that tend to result in flakiness (e.g. sleeps for 200ms and 1000ms) and master itself is failing, so I'll assume that's the reason :)

[fwouts]

Glad to see Cypress tests now seem more stable!

@michaeltaranto FYI this is now up-to-date with master :)

[fwouts]

Friendly ping 🏓

[fwouts]

Hello 👋

[michaeltaranto]

Apologies for taking so long to come back to this. This is a tricky one, as filtering of consumers exports is not really something that feels right to be maintained by Playroom itself.

In practice we have found that pointing Playroom at a file that exports only what you want to be available is becoming more the encouraged approach.

That feels like a less opinionated path forward to me.

[fwouts]

No worries, I can see the reasoning. How about just filtering out undefined at least to reduce the confusion when it crashes?

[michaeltaranto]

Yeah maybe that makes more sense as a defensive approach. Might be worth memoizing that in renderPlayroom function so that the filtering is only re-run when/if components changes.

Thoughts?

[fwouts]

Sorry for the delay, I've updated the PR. Not sure it's worth the additional code for memoizing but it's easy to add if you'd like :)

[markdalgleish]

Thanks for the PR!

[fwouts]

And thanks for the prompt release! :)