3 ms·

Why low quality?

by izietto 5y ago

Why low quality?

Jamie9912 5y ago
46k lines of code for this
- izietto 5y ago
  I count 213 JS + 41 CSS = 254 lines
  - naoqj 5y ago
    You forget the dependencies.
    - CapsAdmin 5y ago
      of course.. :-) I'm not sure what we're talking about here because it's not very obvious where this number comes from. Is it the YouTube API, excluding React? If you're thinking of using a React library, we have to assume you're already using React. Mentioning line count is useless unless you're someone intending to explicitly manage that code. But if so, why stop at 40k? It seems more constructive to talk about the size impact in megabytes or loading time of requiring the YouTube API just to have video playing in the background vs just just hosting your own static video alongside the html. Of course assuming you already use React.
  - 11235813213455 5y ago
    https://packagephobia.com/result?p=youtube-bg-react https://packagephobia.com/result?p=youtube-bg-react
- izietto 5y ago
  If I should point to something that isn't very good is the placeholder test, but the code is ok to me.
ht85 5y ago
- no documentation, not even a few lines - no example without cloning - example provided doesn't run without modifying it (empty video url) - no tests - doesn't look maintained (e.g. 3 years old typescript version) Then looking at index.tsx - using Class as component (debatable but really not idiomatic anymore) - weird empty lines and spaces - weird `this.container` pattern, not to mention the type of it is unsafe - `aspectRatio` has to be in `x:y` format (including the colon), no way to know that unless you read the code in depth (also weird choice) - using the anti-pattern of setting derived state (aspectRatio) in this.state, rendering again with a different aspect ratio won't work - componentWillUnmount does not remove the listener, as calling `.bind` there will create a function that doesn't match the one in componentDidMount - using `.bind` inline insted of using fat arrows directly when creating class methods - the weird syntax for `playerProps =` - hard coding some of the `PlayerVars` props instead of just makin props `PlayerVars & { ... }` to allow passing any props supported by react-youtube Anyway that's just at a glance, I'm sure cloning the project and trying to use it would reveal a lot more.
- deleted 5y ago
  [deleted]

Why low quality?

Jamie9912 5y ago

46k lines of code for this

izietto 5y ago
I count 213 JS + 41 CSS = 254 lines
- naoqj 5y ago
  You forget the dependencies.
  - CapsAdmin 5y ago
    of course.. :-) I'm not sure what we're talking about here because it's not very obvious where this number comes from. Is it the YouTube API, excluding React? If you're thinking of using a React library, we have to assume you're already using React. Mentioning line count is useless unless you're someone intending to explicitly manage that code. But if so, why stop at 40k? It seems more constructive to talk about the size impact in megabytes or loading time of requiring the YouTube API just to have video playing in the background vs just just hosting your own static video alongside the html. Of course assuming you already use React.
- 11235813213455 5y ago
  https://packagephobia.com/result?p=youtube-bg-react https://packagephobia.com/result?p=youtube-bg-react
izietto 5y ago
If I should point to something that isn't very good is the placeholder test, but the code is ok to me.

ht85 5y ago

- no documentation, not even a few lines - no example without cloning - example provided doesn't run without modifying it (empty video url) - no tests - doesn't look maintained (e.g. 3 years old typescript version) Then looking at index.tsx - using Class as component (debatable but really not idiomatic anymore) - weird empty lines and spaces - weird `this.container` pattern, not to mention the type of it is unsafe - `aspectRatio` has to be in `x:y` format (including the colon), no way to know that unless you read the code in depth (also weird choice) - using the anti-pattern of setting derived state (aspectRatio) in this.state, rendering again with a different aspect ratio won't work - componentWillUnmount does not remove the listener, as calling `.bind` there will create a function that doesn't match the one in componentDidMount - using `.bind` inline insted of using fat arrows directly when creating class methods - the weird syntax for `playerProps =` - hard coding some of the `PlayerVars` props instead of just makin props `PlayerVars & { ... }` to allow passing any props supported by react-youtube Anyway that's just at a glance, I'm sure cloning the project and trying to use it would reveal a lot more.

deleted 5y ago
[deleted]