Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

fix isReactComponent returning true for blank input #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bmaland
Copy link

@bmaland bmaland commented Apr 1, 2019

isReactComponent returned true for blank inputs (e.g. undefined), which could produce confusing error messages. E.g. from Vue if you had components: {MyComponent: undefined} (could be undefined because of e.g. a failing import) it would display a Vuera ReactWrapper error instead of the normal Vue "[Vue warn]: Unknown custom element" error.

Copy link
Owner

@akxcv akxcv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello and thank you for this pull request! There are a couple of questions I have about the code, but they should be easy enough to resolve.

@@ -1,4 +1,5 @@
export default function isReactComponent (component) {
if (!component) return false
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be better to write this as

if (component === undefined || component === null || component === false)

For example, I'm not sure if this is correct behaviour for cases when component is '' or 0.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for your reply. I think '' and 0 shouldn't be considered valid React components either so it might be ok to leave it as is?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is, it works both ways because the React wrapper is also using this function. Actually, with your fix it's the React side that will suffer from similar errors. Instead, we should probably just leave "falsy" components as they were. Shouldn't be difficult to do, do you think you can find the time to implement this?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I meant not wrapping falsy components whatsoever.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants