Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix undefined access on not-existing segmentation layer #5583

Merged
merged 4 commits into from
Jun 24, 2021
Merged

Fix undefined access on not-existing segmentation layer #5583

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5ad869
fix undefined access on not-existing segmentation layer
philippotto Jun 22, 2021
640a2aa
update changelog
philippotto Jun 22, 2021
c030dc7
Update CHANGELOG.unreleased.md
philippotto Jun 24, 2021
b2037d5
Merge branch 'master' into fix-undefined-mesh
philippotto Jun 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/view/layouting/default_layout_configs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// @flow
import _ from "lodash";

import type { ExtractReturn } from "libs/type_helpers";
import { getIsInIframe } from "libs/utils";
import { navbarHeight } from "navbar";
import Constants, {
Expand Down Expand Up @@ -298,7 +299,6 @@ export const resetDefaultLayouts = () => {
getDefaultLayouts.cache.clear();
};

type ExtractReturn<Fn> = $Call<<T>(() => T) => T, Fn>;
type Layout = $Keys<ExtractReturn<typeof _getDefaultLayouts>>;

export const getCurrentDefaultLayoutConfig = () => {
Expand Down
4 changes: 1 addition & 3 deletions frontend/javascripts/oxalis/view/node_context_menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,7 @@ function NoNodeContextMenuOptions({
}: NoNodeContextMenuProps) {
useEffect(() => {
(async () => {
if (segmentationLayer) {
await maybeFetchMeshFiles(segmentationLayer, dataset, false);
}
await maybeFetchMeshFiles(segmentationLayer, dataset, false);
})();
}, []);

Expand Down
16 changes: 10 additions & 6 deletions frontend/javascripts/oxalis/view/right-menu/meshes_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const stlIsosurfaceConstants = {

// This file defines the component MeshesView.

const mapStateToProps = (state: OxalisState) => ({
const mapStateToProps = (state: OxalisState): * => ({
Copy link
Member Author

@philippotto philippotto Jun 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a while to figure out why the errors regarding the Props were not caught by flow. It turns out, flow simply assumed the props to be any. The reason for this is that the extraction of the return type for these functions here didn't work. After a bit of digging I found out that the * as a return type helps 🤦

Here's the comment: facebook/flow#7071 (comment)
The issue was opened 2.5 years ago. Yet another reason to ditch flow?

I also looked for ExtractReturn and there is only one other usage which doesn't seem to suffer from the same bug. So, hopefully no other bad suprises.

meshes: state.tracing != null ? state.tracing.meshes : [],
isImporting: state.uiInformation.isImportingMesh,
isosurfaces: state.isosurfaces,
Expand All @@ -75,7 +75,7 @@ const mapStateToProps = (state: OxalisState) => ({
currentMeshFile: state.currentMeshFile,
});

const mapDispatchToProps = (dispatch: Dispatch<*>) => ({
const mapDispatchToProps = (dispatch: Dispatch<*>): * => ({
updateRemoteMeshMetadata(id: string, meshMetaData: $Shape<RemoteMeshMetaData>) {
dispatch(updateRemoteMeshMetaDataAction(id, meshMetaData));
},
Expand Down Expand Up @@ -121,8 +121,9 @@ const mapDispatchToProps = (dispatch: Dispatch<*>) => ({
});

type DispatchProps = ExtractReturn<typeof mapDispatchToProps>;
type StateProps = ExtractReturn<typeof mapStateToProps>;

type Props = {| ...DispatchProps |};
type Props = {| ...DispatchProps, ...StateProps |};

type State = { hoveredListItem: ?number };

Expand Down Expand Up @@ -257,6 +258,9 @@ class MeshesView extends React.Component<Props, State> {
Toast.info("No cell found at centered position");
return;
}
if (!this.props.currentMeshFile || !this.props.segmentationLayer) {
return;
}
await loadMeshFromFile(
id,
pos,
Expand Down Expand Up @@ -295,12 +299,12 @@ class MeshesView extends React.Component<Props, State> {
);

const getLoadPrecomputedMeshButton = () => {
const hasCurrenMeshFile = this.props.currentMeshFile;
const hasCurrentMeshFile = this.props.currentMeshFile;
return (
<Tooltip
key="tooltip"
title={
hasCurrenMeshFile
this.props.currentMeshFile != null
? `Load mesh for centered cell from file ${this.props.currentMeshFile}`
: "There is no mesh file."
}
Expand All @@ -312,7 +316,7 @@ class MeshesView extends React.Component<Props, State> {
overlay={getMeshFilesDropdown()}
buttonsRender={([leftButton, rightButton]) => [
React.cloneElement(leftButton, {
disabled: !hasCurrenMeshFile,
disabled: !hasCurrentMeshFile,
style: { borderRightStyle: "dashed" },
}),
React.cloneElement(rightButton, { style: { borderLeftStyle: "dashed" } }),
Expand Down
5 changes: 4 additions & 1 deletion frontend/javascripts/oxalis/view/right-menu/meshes_view_helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ import processTaskWithPool from "libs/task_pool";
const PARALLEL_MESH_LOADING_COUNT = 6;

export async function maybeFetchMeshFiles(
segmentationLayer: APIDataLayer,
segmentationLayer: ?APIDataLayer,
dataset: APIDataset,
mustRequest: boolean,
): Promise<void> {
if (!segmentationLayer) {
return;
}
const files = Store.getState().availableMeshFiles;

// only send new get request, if it hasn't happened before (files in store are null)
Expand Down