bsidelinger912 / shiitake

React Line clamp that won't get you fired
MIT License
100 stars 22 forks source link

Cannot read property 'length' of null #17

Closed apandichi closed 6 years ago

apandichi commented 6 years ago

Given the following:

const text = null;
<Shiitake lines={2}>{text}</Shiitake>

Throws the following error:

Uncaught (in promise) TypeError: Cannot read property 'length' of null
    at Shiitake.render (bundle-program-builder.js?compile=false:76149)

Sometimes text comes from an API as null. There is a workaround:

const text = null;
<Shiitake lines={2}>{text ? text : ''}</Shiitake>

But I'd rather have the Shiitake component handle this for me instead.

Also, I noticed there is no unit test in shiitake.spec.js for this particular case. I would have sent a pull request, but I could not run any tests on my local machine. Maybe you could add some info in README.md about how to run tests and what dev dependencies are needed for this.

bsidelinger912 commented 6 years ago

Yes, I definitely need to to do a better check to make sure children is a string. Clearly I also need to get the readme set up for contributors as well. I'll address this ASAP, thanks for pointing it out.

bsidelinger912 commented 6 years ago

Okay @apandichi v2.2.2 is updated to handle null and other all non-string values by falling back to an empty string. Let me know when you have a chance to check it out.

apandichi commented 6 years ago

Now I see a warning:

Warning: Failed prop type: The prop `children` is marked as required in `Shiitake`, but its value is `null`.

But other than that, it's fine. Thanks a lot!

bsidelinger912 commented 6 years ago

Yes, this is by design, it uses the PropTypes library. Shiitake requires a string as children and the PropTypes library gives us that warning when one isn't passed.