MemeLabs / Rustla2

Livestream viewing with chat
https://strims.gg
MIT License
44 stars 24 forks source link

StreamThumbnail gets too many properties #12

Closed kenany closed 7 years ago

kenany commented 7 years ago
Warning: Unknown props `viewers`, `created_at`, `updated_at` on <div> tag. Remove these props from the element.
    in div (created by StreamThumbnail)
    in StreamThumbnail (created by Streams)
    in div (created by Streams)
    in div (created by Streams)
    in div (created by Streams)
    in div (created by MainLayout)
    in div (created by MainLayout)
    in MainLayout (created by Streams)
    in Streams (created by lifecycle(Streams))
    in lifecycle(Streams) (created by Connect(lifecycle(Streams)))
    in Connect(lifecycle(Streams)) (created by RouterContext)
    in LoadingScreen (created by Connect(LoadingScreen))
    in Connect(LoadingScreen) (created by RouterContext)
    in RouterContext (created by Router)
    in Router
    in Provider

We still want backend to provide viewers (for Stream), so that has to be handled client-side (don't send to StreamThumbnail). created_at and updated_at don't need to sent at all, however, since it's not used in the frontend, so that should be fixed in the server-side.

kenany commented 7 years ago

Could be done completely client-side, but perhaps this is a cop-out?

diff --git a/src/components/StreamThumbnail.jsx b/src/components/StreamThumbnail.jsx
index 4136675..3e0740d 100644
--- a/src/components/StreamThumbnail.jsx
+++ b/src/components/StreamThumbnail.jsx
@@ -32,7 +32,6 @@ StreamThumbnail.propTypes = {
   service: PropTypes.string.isRequired,
   thumbnail: PropTypes.string,
   live: PropTypes.bool.isRequired,
-  viewers: PropTypes.number,
   rustlers: PropTypes.number.isRequired,
   twitch_channel_id: PropTypes.number,
 };
diff --git a/src/components/Streams.jsx b/src/components/Streams.jsx
index 92874e5..e57264c 100644
--- a/src/components/Streams.jsx
+++ b/src/components/Streams.jsx
@@ -2,6 +2,7 @@ import React, { PropTypes } from 'react';
 import { compose } from 'redux';
 import { connect } from 'react-redux';
 import lifecycle from 'recompose/lifecycle';
+import pick from 'lodash/pick';

 import { setStream } from '../actions';

@@ -25,10 +26,17 @@ const makeCategories = (categories, items) => {
     sortedStreams[i].length ?
     <div key={i}>
       <h3 className='col-xs-12'>{header}</h3>
-      {sortedStreams[i].map(stream =>
-        <div className='col-xs-12 col-sm-4 col-md-3 col-lg-2' key={stream.id}>
-          <StreamThumbnail {...stream} />
-        </div>
+      {sortedStreams[i].map(stream => {
+        // Don't send stream properties that aren't required by StreamThumbnail.
+        const strippedStream = pick(stream,
+          Object.keys(StreamThumbnail.propTypes));
+
+        return (
+          <div className='col-xs-12 col-sm-4 col-md-3 col-lg-2' key={stream.id}>
+            <StreamThumbnail {...strippedStream} />
+          </div>
+        );
+      }
       )}
     </div>
     : null
blushies commented 7 years ago

That'll work, try to use object rest instead of pick

kenany commented 7 years ago

Good thing about pick here is that we wouldn't have to hard-code the required properties. Hopefully I'm just missing something here because otherwise things get ugly.

If I do it like this, we gotta list the required properties twice:

        // Don't send stream properties that aren't required by StreamThumbnail.
        const {
          channel,
          live,
          rustlers,
          service,
          thumbnail,
        } = stream;

        return (
          <div className='col-xs-12 col-sm-4 col-md-3 col-lg-2' key={stream.id}>
            <StreamThumbnail
              {...{
                channel,
                live,
                rustlers,
                service,
                thumbnail }}
              />
          </div>
        );

If I do it this way, we introduce unused variables which trigger warnings (which, of course, I could turn off for this block):

const { viewers, created_at, updated_at, ...rest } = stream;
kenany commented 7 years ago

Settled on

// Don't send stream properties that aren't required by StreamThumbnail.
// eslint-disable-next-line no-unused-vars
const { created_at, updated_at, viewers, ...rest } = stream;