PublishFile: fix render function violation (#1518)

* PublishFile: fix render function violation

Per doc:
> A React component should not cause side effects in other components during rendering.

Even in own render function (allowed to call), it should be avoided as it could cause infinite loops.

* PublishFile: fix useEffect infinite loop due to bad dependency

## Issue
One of the effects was adding an internal wrapper function as a dependency. As this is a functional component, the wrapper is re-created on every render and would spark the effect. That effect also updates redux (depending on the code path), so we end up in a loop.

## Change 1
Two options to fix the dependency:
1. Just remove the wrappers from the list, since we "know" it is essentially the same function (i.e. it's not function-variable that could point to something else at runtime).
2. Peek into the wrapper and determine what are the actual dependencies (usually props or data derived from props).

Solution 2 is the norm.

Aside: wrappers are usually the root-cause of incorrect dependencies, because they mask away the actual code. Need to always peek into it.

## Change 2
Next, change the dispatch-to-props map from function version to object version so that we have stable references to the actions. The object version is also preferred when we don't need to make any customizations to the actions.
This commit is contained in:
infinite-persistence 2022-05-19 22:41:32 +08:00 committed by GitHub
parent 0998e3d48c
commit b733215c5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 22 deletions

View file

@ -23,10 +23,10 @@ const select = (state, props) => ({
isLivestreamClaim: selectIsStreamPlaceholderForUri(state, props.uri), isLivestreamClaim: selectIsStreamPlaceholderForUri(state, props.uri),
}); });
const perform = (dispatch) => ({ const perform = {
clearPublish: () => dispatch(doClearPublish()), doClearPublish,
updatePublishForm: (value) => dispatch(doUpdatePublishForm(value)), doUpdatePublishForm,
showToast: (message) => dispatch(doToast({ message, isError: true })), doToast,
}); };
export default connect(select, perform)(PublishPage); export default connect(select, perform)(PublishPage);

View file

@ -30,12 +30,12 @@ type Props = {
fileMimeType: ?string, fileMimeType: ?string,
isStillEditing: boolean, isStillEditing: boolean,
balance: number, balance: number,
updatePublishForm: ({}) => void, doUpdatePublishForm: ({}) => void,
disabled: boolean, disabled: boolean,
publishing: boolean, publishing: boolean,
showToast: (string) => void, doToast: ({ message: string, isError?: boolean }) => void,
inProgress: boolean, inProgress: boolean,
clearPublish: () => void, doClearPublish: () => void,
ffmpegStatus: any, ffmpegStatus: any,
optimize: boolean, optimize: boolean,
size: number, size: number,
@ -68,11 +68,12 @@ function PublishFile(props: Props) {
filePath, filePath,
fileMimeType, fileMimeType,
isStillEditing, isStillEditing,
updatePublishForm, doUpdatePublishForm: updatePublishForm,
doToast,
disabled, disabled,
publishing, publishing,
inProgress, inProgress,
clearPublish, doClearPublish,
optimize, optimize,
ffmpegStatus = {}, ffmpegStatus = {},
size, size,
@ -117,6 +118,9 @@ function PublishFile(props: Props) {
limit: TV_PUBLISH_SIZE_LIMIT_GB_STR, limit: TV_PUBLISH_SIZE_LIMIT_GB_STR,
}); });
const bitRate = getBitrate(size, duration);
const bitRateIsOverMax = bitRate > MAX_BITRATE;
const fileSelectorModes = [ const fileSelectorModes = [
{ label: __('Upload'), actionName: SOURCE_UPLOAD, icon: ICONS.PUBLISH }, { label: __('Upload'), actionName: SOURCE_UPLOAD, icon: ICONS.PUBLISH },
{ label: __('Choose Replay'), actionName: SOURCE_SELECT, icon: ICONS.MENU }, { label: __('Choose Replay'), actionName: SOURCE_SELECT, icon: ICONS.MENU },
@ -202,7 +206,7 @@ function PublishFile(props: Props) {
handleFileChange(filePath); handleFileChange(filePath);
} }
} }
}, [filePath, currentFile, handleFileChange, updateFileInfo]); }, [filePath, currentFile, doToast, updatePublishForm]);
useEffect(() => { useEffect(() => {
const isOptimizeAvail = currentFile && currentFile !== '' && isVid && ffmpegAvail; const isOptimizeAvail = currentFile && currentFile !== '' && isVid && ffmpegAvail;
@ -212,6 +216,10 @@ function PublishFile(props: Props) {
updatePublishForm({ optimize: finalOptimizeState }); updatePublishForm({ optimize: finalOptimizeState });
}, [currentFile, filePath, isVid, ffmpegAvail, userOptimize, updatePublishForm]); }, [currentFile, filePath, isVid, ffmpegAvail, userOptimize, updatePublishForm]);
useEffect(() => {
setOverMaxBitrate(bitRateIsOverMax);
}, [bitRateIsOverMax]);
function updateFileInfo(duration, size, isvid) { function updateFileInfo(duration, size, isvid) {
updatePublishForm({ fileDur: duration, fileSize: size, fileVid: isvid }); updatePublishForm({ fileDur: duration, fileSize: size, fileVid: isvid });
} }
@ -264,18 +272,11 @@ function PublishFile(props: Props) {
); );
} }
// @endif // @endif
let bitRate = getBitrate(size, duration);
let overMaxBitrate = bitRate > MAX_BITRATE;
if (overMaxBitrate) {
setOverMaxBitrate(true);
} else {
setOverMaxBitrate(false);
}
if (isVid && duration && bitRate > RECOMMENDED_BITRATE) { if (isVid && duration && bitRate > RECOMMENDED_BITRATE) {
return ( return (
<p className="help--warning"> <p className="help--warning">
{overMaxBitrate {bitRateIsOverMax
? __( ? __(
'Your video has a bitrate over ~12 Mbps and cannot be processed at this time. We suggest transcoding to provide viewers the best experience.' 'Your video has a bitrate over ~12 Mbps and cannot be processed at this time. We suggest transcoding to provide viewers the best experience.'
) )
@ -392,7 +393,6 @@ function PublishFile(props: Props) {
} }
function handleFileChange(file: WebFile, clearName = true) { function handleFileChange(file: WebFile, clearName = true) {
const { showToast } = props;
window.URL = window.URL || window.webkitURL; window.URL = window.URL || window.webkitURL;
setOversized(false); setOversized(false);
setOverMaxBitrate(false); setOverMaxBitrate(false);
@ -459,7 +459,7 @@ function PublishFile(props: Props) {
// we only need to enforce file sizes on 'web' // we only need to enforce file sizes on 'web'
if (file.size && Number(file.size) > TV_PUBLISH_SIZE_LIMIT_BYTES) { if (file.size && Number(file.size) > TV_PUBLISH_SIZE_LIMIT_BYTES) {
setOversized(true); setOversized(true);
showToast(__(UPLOAD_SIZE_MESSAGE)); doToast({ message: __(UPLOAD_SIZE_MESSAGE), isError: true });
updatePublishForm({ filePath: '' }); updatePublishForm({ filePath: '' });
return; return;
} }
@ -498,7 +498,7 @@ function PublishFile(props: Props) {
button="close" button="close"
label={__('New --[clears Publish Form]--')} label={__('New --[clears Publish Form]--')}
icon={ICONS.REFRESH} icon={ICONS.REFRESH}
onClick={clearPublish} onClick={doClearPublish}
/> />
</div> </div>
)} )}