Skip to content

Commit

Permalink
Ensure exported components don't reference the URL directly (#660)
Browse files Browse the repository at this point in the history
* Add UI typedefs job

* Avoid using URL state in exported components
  • Loading branch information
jpellizzari committed Aug 30, 2021
1 parent c06e834 commit 8070afb
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 34 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Expand Up @@ -41,6 +41,6 @@ jobs:
registry-url: "https://npm.pkg.github.com"
scope: "@weaveworks"
- run: npm install
- run: make dist/index.js && cd dist && npm publish
- run: make ui-lib && cd dist && npm publish
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
8 changes: 7 additions & 1 deletion Makefile
Expand Up @@ -93,11 +93,17 @@ ui-audit:

ui: node_modules cmd/wego/ui/run/dist/main.js

ui-lib: node_modules dist/index.js
ui-lib: node_modules dist/index.js dist/index.d.ts
# Remove font files from the npm module.
@find dist -type f -iname \*.otf -delete
@find dist -type f -iname \*.woff -delete

dist/index.js:
npm run build:lib && cp package.json dist

dist/index.d.ts:
npm run typedefs

# JS coverage info
coverage/lcov.info:
npm run test -- --coverage
Expand Down
4 changes: 3 additions & 1 deletion package.json
Expand Up @@ -6,6 +6,7 @@
"preinstall": "npm install --package-lock-only --ignore-scripts && npx npm-force-resolutions",
"build": "parcel build --no-source-maps ui/index.html --dist-dir cmd/wego/ui/run/dist",
"build:lib": "parcel build --no-source-maps ui/index.ts --dist-dir dist",
"typedefs": "npx -p typescript tsc ui/**/*.ts ui/**/*.tsx --skipLibCheck --esModuleInterop --jsx react-jsx --declaration --allowJs --emitDeclarationOnly --outDir dist",
"start": "parcel serve --port 4567 ui/index.html",
"lint": "eslint ui",
"test": "jest",
Expand Down Expand Up @@ -49,7 +50,8 @@
"moduleNameMapper": {
"\\.(jpg|ico|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/ui/lib/fileMock.js",
"\\.(css|less)$": "<rootDir>/ui/lib/fileMock.js"
}
},
"modulePathIgnorePatterns": ["<rootDir>/dist/"]
},
"resolutions": {
"postcss": "8.3.6",
Expand Down
7 changes: 6 additions & 1 deletion ui/App.tsx
@@ -1,4 +1,5 @@
import { MuiThemeProvider } from "@material-ui/core";
import qs from "query-string";
import * as React from "react";
import {
BrowserRouter as Router,
Expand Down Expand Up @@ -36,7 +37,11 @@ export default function App() {
<Route
exact
path={PageRoute.ApplicationDetail}
component={ApplicationDetail}
component={({ location }) => {
const params = qs.parse(location.search);

return <ApplicationDetail name={params.name as string} />;
}}
/>
<Redirect exact from="/" to={PageRoute.Applications} />
<Route exact path="*" component={Error} />
Expand Down
13 changes: 6 additions & 7 deletions ui/hooks/__tests__/navigation.test.tsx
Expand Up @@ -13,18 +13,17 @@ describe("useNavigation", () => {
document.body.removeChild(container);
container = null;
});

it("returns the query", () => {
it("displays the current page", () => {
const id = "custom-element";
const myVar = "myVar";
const myPage = "my_page";
const TestComponent = () => {
const { query } = useNavigation<{ someKey: string }>();
const { currentPage } = useNavigation();

return <p data-testid={id}>{query.someKey}</p>;
return <p data-testid={id}>{currentPage}</p>;
};

render(withContext(TestComponent, `/?someKey=${myVar}`));
render(withContext(TestComponent, `/${myPage}`));

expect(screen.getByTestId(id).textContent).toEqual(myVar);
expect(screen.getByTestId(id).textContent).toEqual(myPage);
});
});
18 changes: 2 additions & 16 deletions ui/hooks/navigation.ts
@@ -1,20 +1,14 @@
import _ from "lodash";
import qs from "query-string";
import { useEffect, useState } from "react";
import { useHistory, useLocation } from "react-router-dom";
import { PageRoute } from "../lib/types";
import { formatURL } from "../lib/utils";
import { useLocation } from "react-router-dom";

export const normalizePath = (pathname) => {
return _.tail(pathname.split("/"));
};

export default function useNavigation<T>(): {
export default function useNavigation(): {
currentPage: string;
query: T;
navigate: (PageRoute, any) => void;
} {
const history = useHistory();
const location = useLocation();
const [currentPage, setCurrentPage] = useState("");

Expand All @@ -23,15 +17,7 @@ export default function useNavigation<T>(): {
setCurrentPage(pageName as string);
}, [location]);

const navigate = (page: PageRoute, query: any) => {
history.push(formatURL(page, query));
};

const q = qs.parse(location.search) as any;

return {
currentPage,
query: q,
navigate,
};
}
4 changes: 2 additions & 2 deletions ui/lib/theme.ts
@@ -1,6 +1,6 @@
// Typescript will handle type-checking/linting for this file
/* eslint-disable */
import { createMuiTheme } from "@material-ui/core";
import { createTheme } from "@material-ui/core";
import { createGlobalStyle, DefaultTheme } from "styled-components";
// @ts-ignore
import ProximaNovaBold from "url:../fonts/ProximaNovaBold.otf";
Expand Down Expand Up @@ -90,6 +90,6 @@ export const GlobalStyle = createGlobalStyle`
}
`;

export const muiTheme = createMuiTheme({
export const muiTheme = createTheme({
typography: { fontFamily: "proxima-nova" },
});
7 changes: 2 additions & 5 deletions ui/pages/ApplicationDetail.tsx
Expand Up @@ -4,19 +4,16 @@ import ConditionsTable from "../components/ConditionsTable";
import KeyValueTable from "../components/KeyValueTable";
import Page from "../components/Page";
import useApplications from "../hooks/applications";
import useNavigation from "../hooks/navigation";
import { Application } from "../lib/api/applications/applications.pb";
import { PageRoute } from "../lib/types";

type Props = {
className?: string;
name: string;
};

function ApplicationDetail({ className }: Props) {
function ApplicationDetail({ className, name }: Props) {
const [app, setApp] = React.useState<Application>({});
const {
query: { name },
} = useNavigation<{ name: string }>();

const { getApplication, loading } = useApplications();

Expand Down

0 comments on commit 8070afb

Please sign in to comment.