Fix some react warnings #5877
Fix some react warnings #5877
Conversation
Codecov Report
@@ Coverage Diff @@
## master mozilla/addons-frontend#5877 +/- ##
==========================================
+ Coverage 97.72% 97.73% +<.01%
==========================================
Files 229 229
Lines 5859 5860 +1
Branches 1122 1122
==========================================
+ Hits 5726 5727 +1
Misses 118 118
Partials 15 15
Continue to review full report at Codecov.
|
willdurand
force-pushed
the
fix-server-invalid-props
branch
from
93deab6
to
f466c57
Compare
src/amo/components/Addon/index.js
Outdated
| @@ -60,8 +60,8 @@ const slugIsPositiveID = (slug) => { | |||
| export class AddonBase extends React.Component { | |||
| static propTypes = { | |||
| config: PropTypes.object, | |||
| RatingManager: PropTypes.element, | |||
| addon: PropTypes.object.isRequired, | |||
| RatingManager: PropTypes.func, | |||
There was a problem hiding this comment.
This is pretty common and there is an issue here: facebook/react#5143. TL;DR: no type for what we want to do, here we need a function though (because we apply compose() to the base component).
| <Tr> | ||
| : Array.from(Array(50)).map((_, i) => ( | ||
| // eslint-disable-next-line react/jsx-indent, react/no-array-index-key | ||
| <Tr key={`LoadingText-${i}`}> |
There was a problem hiding this comment.
We need to specify keys all the time when we use map (or .fill but it is not exactly the same thing).
There was a problem hiding this comment.
yeh i think any child in an array or iterator we do, right?
| @@ -21,7 +21,7 @@ export function getNoScriptStyles( | |||
| `src/${appName}/noscript.css`, | |||
| ); | |||
| try { | |||
| return fs.readFileSync(cssPath); | |||
| return fs.readFileSync(cssPath, 'utf8'); | |||
There was a problem hiding this comment.
Don't recall the error but we should really pass the encoding in readFileSync()
willdurand
force-pushed
the
fix-server-invalid-props
branch
from
f466c57
to
52ce100
Compare
willdurand
force-pushed
the
fix-server-invalid-props
branch
from
52ce100
to
856d882
Compare
| @@ -60,8 +60,8 @@ const slugIsPositiveID = (slug) => { | |||
|
|
|||
| export class AddonBase extends React.Component { | |||
| static propTypes = { | |||
| RatingManager: PropTypes.element, | |||
| addon: PropTypes.object.isRequired, | |||
| RatingManager: PropTypes.func, | |||
There was a problem hiding this comment.
hmm, didn't expect that but seems this is ok..
| <Tr> | ||
| : Array.from(Array(50)).map((_, i) => ( | ||
| // eslint-disable-next-line react/jsx-indent, react/no-array-index-key | ||
| <Tr key={`LoadingText-${i}`}> |
There was a problem hiding this comment.
yeh i think any child in an array or iterator we do, right?
|
actually I am seeing this still when I run disco: Warning: Failed prop type: The prop `handleGlobalEvent` is marked as required in `DiscoPaneBase`, but its value is `undefined`.
[1] in DiscoPaneBase
[1] in Translate(DiscoPaneBase)
[1] in Connect(Translate(DiscoPaneBase))
...
[1] Warning: Failed prop type: The prop `hasAddonManager` is marked as required in `AddonBase`, but its value is `undefined`.
[1] in AddonBase
[1] in WithInstallHelpers
...
[1] Warning: Failed prop type: The prop `status` is marked as required in `AddonBase`, but its value is `undefined`.
[1] in AddonBase
[1] in WithInstallHelpers
[1] in Connect(WithInstallHelpers)
...are you not seeing these? |
|
I'll follow up with the disco pane, which is being modified in other PRs. |
Fix mozilla/addons#12172
This PR aims at fixing most of the issues displayed when running
yarn amo:dev.The
InfoDialogwarnings are fixed in #5880