HPCC-32594 ECL Watch v9 prevent duplicate errors logged on File Details #19094
Conversation
fixes an issue where the Files Details page would potentially log multiple duplicate errors due to the interdependence of the various file hooks (which all consume useFile). also, sets default values for name and image in the LogicalFileSummary component for the scenario of viewing a deleted file. Signed-off-by: Jeremy Clements <[email protected]>
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-32594 Jirabot Action Result: |
| return <_.FileDetails cluster={undefined} logicalFile={params.Name as string} />; | ||
| }) | ||
| path: "/:Name", action: (ctx, params) => { | ||
| const FileContextProvider = React.lazy(() => import("./components/contexts/FileContext")); |
There was a problem hiding this comment.
This is a bit different, but I wasn't quite sure what would be better for conditionally loading more than one component in these router callbacks. I found this React.lazy in the docs, and it required wrapping in this React.Suspense to render properly. lazy() also expects the component being loaded to have a default export, hence the one line change to FileDetails.
There was a problem hiding this comment.
My understanding is that the only diff between a raw dynamic import and the React.lazy, is the facility to supply a "loading" message?
From our point of view, ensuring that these are bundled into individual "chunks" and loaded "on demand" is v. important as it facilitates the gradual migration away from Dojo (by better encapsulation)
| return context; | ||
| }; | ||
|
|
||
| export default FileContextProvider; |
There was a problem hiding this comment.
Picky: For consistency, there is no need to have a default export...
| @@ -156,3 +156,5 @@ export const FileDetails: React.FunctionComponent<FileDetailsProps> = ({ | |||
| </div> | |||
| }</SizeMe>; | |||
| }; | |||
|
|
|||
| export default FileDetails; | |||
There was a problem hiding this comment.
Picky: For consistency, there is no need to have a default export...
| return <_.FileDetails cluster={undefined} logicalFile={params.Name as string} />; | ||
| }) | ||
| path: "/:Name", action: (ctx, params) => { | ||
| const FileContextProvider = React.lazy(() => import("./components/contexts/FileContext")); |
There was a problem hiding this comment.
My understanding is that the only diff between a raw dynamic import and the React.lazy, is the facility to supply a "loading" message?
From our point of view, ensuring that these are bundled into individual "chunks" and loaded "on demand" is v. important as it facilitates the gradual migration away from Dojo (by better encapsulation)
| <FileDetails cluster={undefined} logicalFile={params.Name as string} /> | ||
| </FileContextProvider> | ||
| </React.Suspense>; | ||
| } |
There was a problem hiding this comment.
This looks like an "anti-pattern"? Why is FileDetails visible here?
| <FileDetails cluster={params.NodeGroup as string} logicalFile={params.Name as string} /> | ||
| </FileContextProvider> | ||
| </React.Suspense>; | ||
| } |
| <FileDetails cluster={params.NodeGroup as string} logicalFile={params.Name as string} tab={params.Tab as string} sort={{ [params.Tab as string]: parseSort(ctx.search) }} queryParams={{ [params.Tab as string]: parseSearch(ctx.search) as any }} /> | ||
| </FileContextProvider> | ||
| </React.Suspense>; | ||
| } |
fixes an issue where the Files Details page would potentially log multiple duplicate errors due to the interdependence of the various file hooks (which all consume useFile).
also, sets default values for name and image in the LogicalFileSummary component for the scenario of viewing a deleted file.
Type of change:
Checklist:
Smoketest:
Testing: