Skip to content

Commit

Permalink
Remove unsafe methods (#1292)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andres Martinez Gotor authored Nov 19, 2019
1 parent d938f32 commit aaa54a7
Show file tree
Hide file tree
Showing 16 changed files with 113 additions and 61 deletions.
17 changes: 17 additions & 0 deletions dashboard/src/components/AppList/AppList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ const defaultProps: any = {
fetchAppsWithUpdateInfo: jest.fn(),
};

context("when changing props", () => {
it("should fetch apps in the new namespace", () => {
const fetchAppsWithUpdateInfo = jest.fn();
const wrapper = shallow(
<AppList {...defaultProps} fetchAppsWithUpdateInfo={fetchAppsWithUpdateInfo} />,
);
wrapper.setProps({ namespace: "foo" });
expect(fetchAppsWithUpdateInfo).toHaveBeenCalledWith("foo", undefined);
});

it("should update the filter", () => {
const wrapper = shallow(<AppList {...defaultProps} />);
wrapper.setProps({ filter: "foo" });
expect(wrapper.state("filter")).toEqual("foo");
});
});

context("while fetching apps", () => {
props = { ...defaultProps, apps: { isFetching: true } };

Expand Down
10 changes: 5 additions & 5 deletions dashboard/src/components/AppList/AppList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ class AppList extends React.Component<IAppListProps, IAppListState> {
this.setState({ filter });
}

public UNSAFE_componentWillReceiveProps(nextProps: IAppListProps) {
public componentDidUpdate(prevProps: IAppListProps) {
const {
apps: { error, listingAll },
fetchAppsWithUpdateInfo,
filter,
namespace,
} = this.props;
// refetch if new namespace or error removed due to location change
if (nextProps.namespace !== namespace || (error && !nextProps.apps.error)) {
fetchAppsWithUpdateInfo(nextProps.namespace, listingAll);
if (prevProps.namespace !== namespace || (!error && prevProps.apps.error)) {
fetchAppsWithUpdateInfo(namespace, listingAll);
}
if (nextProps.filter !== filter) {
this.setState({ filter: nextProps.filter });
if (prevProps.filter !== filter) {
this.setState({ filter });
}
}

Expand Down
19 changes: 7 additions & 12 deletions dashboard/src/components/AppView/AppView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,26 +81,21 @@ class AppView extends React.Component<IAppViewProps, IAppViewState> {
getAppWithUpdateInfo(releaseName, namespace);
}

// componentWillReceiveProps is deprecated use componentDidUpdate instead
public UNSAFE_componentWillReceiveProps(nextProps: IAppViewProps) {
const { releaseName, getAppWithUpdateInfo, namespace } = this.props;
if (nextProps.namespace !== namespace) {
getAppWithUpdateInfo(releaseName, nextProps.namespace);
return;
}
if (nextProps.error) {
public componentDidUpdate(prevProps: IAppViewProps) {
const { releaseName, getAppWithUpdateInfo, namespace, error, app } = this.props;
if (prevProps.namespace !== namespace) {
getAppWithUpdateInfo(releaseName, namespace);
return;
}
const newApp = nextProps.app;
if (!newApp) {
if (error || !app) {
return;
}

// TODO(prydonius): Okay to use non-safe load here since we assume the
// manifest is pre-parsed by Helm and Kubernetes. Look into switching back
// to safeLoadAll once https://github.com/nodeca/js-yaml/issues/456 is
// resolved.
let manifest: IResource[] = yaml.loadAll(newApp.manifest, undefined, { json: true });
let manifest: IResource[] = yaml.loadAll(app.manifest, undefined, { json: true });
// Filter out elements in the manifest that does not comply
// with { kind: foo }
manifest = manifest.filter(r => r && r.kind);
Expand All @@ -111,7 +106,7 @@ class AppView extends React.Component<IAppViewProps, IAppViewState> {
}

// Iterate over the current manifest to populate the initial state
this.setState(this.parseResources(manifest, newApp.namespace));
this.setState(this.parseResources(manifest, app.namespace));
}

public render() {
Expand Down
8 changes: 7 additions & 1 deletion dashboard/src/components/Catalog/Catalog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,17 @@ it("propagates the filter from the props", () => {
it("reloads charts when the repo changes", () => {
const fetchCharts = jest.fn();
const wrapper = shallow(<Catalog {...defaultProps} fetchCharts={fetchCharts} />);
wrapper.setProps({ ...defaultProps, repo: "bitnami" });
wrapper.setProps({ ...defaultProps, fetchCharts, repo: "bitnami" });
expect(fetchCharts.mock.calls.length).toBe(2);
expect(fetchCharts.mock.calls[1]).toEqual(["bitnami"]);
});

it("updates the filter from props", () => {
const wrapper = shallow(<Catalog {...defaultProps} />);
wrapper.setProps({ filter: "foo" });
expect(wrapper.state("filter")).toBe("foo");
});

describe("renderization", () => {
context("when no charts", () => {
it("should render an error", () => {
Expand Down
10 changes: 5 additions & 5 deletions dashboard/src/components/Catalog/Catalog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ class Catalog extends React.Component<ICatalogProps, ICatalogState> {
fetchCharts(repo);
}

public UNSAFE_componentWillReceiveProps(nextProps: ICatalogProps) {
if (nextProps.filter !== this.state.filter) {
this.setState({ filter: nextProps.filter });
public componentDidUpdate(prevProps: ICatalogProps) {
if (this.props.filter !== this.state.filter) {
this.setState({ filter: this.props.filter });
}
if (this.props.repo !== nextProps.repo) {
this.props.fetchCharts(nextProps.repo);
if (this.props.repo !== prevProps.repo) {
this.props.fetchCharts(this.props.repo);
}
}

Expand Down
8 changes: 4 additions & 4 deletions dashboard/src/components/ChartView/ChartReadme.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ interface IChartReadmeProps {
}

class ChartReadme extends React.Component<IChartReadmeProps> {
public UNSAFE_componentWillMount() {
public componentDidMount() {
const { getChartReadme, version } = this.props;
getChartReadme(version);
}

public UNSAFE_componentWillReceiveProps(nextProps: IChartReadmeProps) {
const { getChartReadme, version } = nextProps;
if (version !== this.props.version) {
public componentDidUpdate(prevProps: IChartReadmeProps) {
const { getChartReadme, version } = this.props;
if (version !== prevProps.version) {
getChartReadme(version);
}
}
Expand Down
6 changes: 3 additions & 3 deletions dashboard/src/components/ChartView/ChartView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ class ChartView extends React.Component<IChartViewProps> {
fetchChartVersionsAndSelectVersion(chartID, version);
}

public UNSAFE_componentWillReceiveProps(nextProps: IChartViewProps) {
public componentDidUpdate(prevProps: IChartViewProps) {
const { selectChartVersion, version } = this.props;
const { versions } = this.props.selected;
if (nextProps.version !== version) {
const cv = versions.find(v => v.attributes.version === nextProps.version);
if (prevProps.version !== version) {
const cv = versions.find(v => v.attributes.version === version);
if (cv) {
selectChartVersion(cv);
} else {
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Config/AppRepoList/AppRepoList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ class AppRepoList extends React.Component<IAppRepoListProps> {
this.props.fetchRepos();
}

public UNSAFE_componentWillReceiveProps(nextProps: IAppRepoListProps) {
public componentDidUpdate(prevProps: IAppRepoListProps) {
const {
errors: { fetch },
fetchRepos,
} = this.props;
// refetch if error removed due to location change
if (fetch && !nextProps.errors.fetch) {
if (prevProps.errors.fetch && !fetch) {
fetchRepos();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ class SliderParam extends React.Component<ISliderParamProps, ISliderParamState>
value: getDefaultValue(this.props.min, this.props.param.value),
};

// tslint:disable-next-line:variable-name
public UNSAFE_componentWillReceiveProps = (props: ISliderParamProps) => {
const value = getDefaultValue(this.props.min, props.param.value);
if (value !== this.state.value) {
this.setState({ value });
public componentDidUpdate = (prevProps: ISliderParamProps) => {
if (prevProps.param.value !== this.props.param.value) {
const value = getDefaultValue(this.props.min, this.props.param.value);
if (value !== this.state.value) {
this.setState({ value });
}
}
};

// onChangeSlider is executed when the slider is dropped at one point
// at that point we update the parameter
public onChangeSlider = (values: number[]) => {
Expand Down
19 changes: 6 additions & 13 deletions dashboard/src/components/DeploymentFormBody/DeploymentFormBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,18 @@ class DeploymentFormBody extends React.Component<
getChartVersion(chartID, chartVersion);
}

// tslint:disable-next-line:variable-name
public UNSAFE_componentWillReceiveProps = (nextProps: IDeploymentFormBodyProps) => {
const { chartID, chartVersion, getChartVersion } = this.props;
public componentDidUpdate = (prevProps: IDeploymentFormBodyProps) => {
const { chartID, chartVersion, getChartVersion, selected, appValues } = this.props;

if (chartVersion !== nextProps.chartVersion) {
if (chartVersion !== prevProps.chartVersion) {
// New version detected
getChartVersion(chartID, nextProps.chartVersion);
getChartVersion(chartID, chartVersion);
return;
}

if (
nextProps.selected.schema !== this.props.selected.schema ||
nextProps.appValues !== this.props.appValues
) {
if (prevProps.selected.schema !== selected.schema || prevProps.appValues !== appValues) {
this.setState({
basicFormParameters: retrieveBasicFormParams(
nextProps.appValues,
nextProps.selected.schema,
),
basicFormParameters: retrieveBasicFormParams(appValues, selected.schema),
});
}
};
Expand Down
7 changes: 7 additions & 0 deletions dashboard/src/components/Header/Header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ it("renders the header links and titles", () => {
});
});

it("updates state when the path changes", () => {
const wrapper = shallow(<Header {...defaultProps} />);
wrapper.setState({ configOpen: true, mobileOpne: true });
wrapper.setProps({ pathname: "foo" });
expect(wrapper.state()).toMatchObject({ configOpen: false, mobileOpen: false });
});

it("renders the namespace switcher", () => {
const wrapper = shallow(<Header {...defaultProps} />);

Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/components/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class Header extends React.Component<IHeaderProps, IHeaderState> {
};
}

public UNSAFE_componentWillReceiveProps(newProps: IHeaderProps) {
if (newProps.pathname !== this.props.pathname) {
public componentDidUpdate(prevProps: IHeaderProps) {
if (prevProps.pathname !== this.props.pathname) {
this.setState({
configOpen: false,
mobileOpen: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ it("renders the warning for alpha feature", () => {
).toContain("Service Catalog integration is under heavy development");
});

context("when changing props", () => {
it("should fetch services in the new namespace", () => {
const getInstances = jest.fn();
const wrapper = shallow(<ServiceInstanceList {...defaultProps} getInstances={getInstances} />);
wrapper.setProps({ namespace: "foo" });
expect(getInstances).toHaveBeenCalledWith("foo");
});

it("should update the filter", () => {
const wrapper = shallow(<ServiceInstanceList {...defaultProps} />);
wrapper.setProps({ filter: "foo" });
expect(wrapper.state("filter")).toEqual("foo");
});
});

context("while fetching components", () => {
const props = { ...defaultProps, classes: { isFetching: true, list: [] } };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ class ServiceInstanceList extends React.PureComponent<
this.setState({ filter: this.props.filter });
}

public UNSAFE_componentWillReceiveProps(nextProps: IServiceInstanceListProps) {
public componentDidUpdate(prevProps: IServiceInstanceListProps) {
const { error, filter, getInstances, isServiceCatalogInstalled, namespace } = this.props;
// refetch if new namespace or error removed due to location change
if (
isServiceCatalogInstalled &&
(nextProps.namespace !== namespace || (error && !nextProps.error))
(prevProps.namespace !== namespace || (error && !prevProps.error))
) {
getInstances(nextProps.namespace);
getInstances(namespace);
}
if (nextProps.filter !== filter) {
this.setState({ filter: nextProps.filter });
if (prevProps.filter !== filter) {
this.setState({ filter });
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ context("while fetching components", () => {
});
});

context("when changing props", () => {
it("should fetch services in the new namespace", () => {
const getInstances = jest.fn();
const getBindings = jest.fn();
const wrapper = shallow(
<ServiceInstanceView
{...defaultProps}
getInstances={getInstances}
getBindings={getBindings}
/>,
);
wrapper.setProps({ namespace: "foo" });
expect(getInstances).toHaveBeenCalledWith("foo");
expect(getBindings).toHaveBeenCalledWith("foo");
});
});

context("when all the components are loaded", () => {
it("shows an error if the requested instance doesn't exist", () => {
const wrapper = shallow(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ class ServiceInstanceView extends React.Component<IServiceInstanceViewProps> {
this.props.getPlans();
}

public UNSAFE_componentWillReceiveProps(nextProps: IServiceInstanceViewProps) {
public componentDidUpdate(prevProps: IServiceInstanceViewProps) {
const { getInstances, getBindings, namespace } = this.props;
if (nextProps.namespace !== namespace) {
getInstances(nextProps.namespace);
getBindings(nextProps.namespace);
if (prevProps.namespace !== namespace) {
getInstances(namespace);
getBindings(namespace);
}
}

Expand Down

0 comments on commit aaa54a7

Please sign in to comment.