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
Open

fix isReactComponent returning true for blank input #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dist/vuera.cjs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ var ReactWrapper = {
};

function isReactComponent(component) {
if (!component) return false;
if ((typeof component === 'undefined' ? 'undefined' : _typeof(component)) === 'object' && !isReactForwardReference(component)) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions dist/vuera.es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ var ReactWrapper = {
};

function isReactComponent(component) {
if (!component) return false;
if ((typeof component === 'undefined' ? 'undefined' : _typeof(component)) === 'object' && !isReactForwardReference(component)) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions dist/vuera.iife.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ var ReactWrapper = {
};

function isReactComponent(component) {
if (!component) return false;
if ((typeof component === 'undefined' ? 'undefined' : _typeof(component)) === 'object' && !isReactForwardReference(component)) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions src/utils/isReactComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.

if (typeof component === 'object' && !isReactForwardReference(component)) {
return false
}
Expand Down
6 changes: 6 additions & 0 deletions tests/utils/isReactComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,10 @@ describe('isReactComponent', () => {
expect(isReactComponent(VueRegisteredComponent)).toBe(false)
expect(isReactComponent(VueSingleFileComponent)).toBe(false)
})

it('returns false for blank input', () => {
expect(isReactComponent(false)).toBe(false)
expect(isReactComponent(null)).toBe(false)
expect(isReactComponent(undefined)).toBe(false)
})
})