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.

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

Remove unsafe methods #1292

Merged
merged 1 commit into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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