diff --git a/apps/web-console/src/app-layout/AppLayout.tsx b/apps/web-console/src/app-layout/AppLayout.tsx index ffafc3d054..6ed7df93c2 100644 --- a/apps/web-console/src/app-layout/AppLayout.tsx +++ b/apps/web-console/src/app-layout/AppLayout.tsx @@ -1,7 +1,7 @@ import React from 'react' import styled from 'styled-components' -import { useApiData, api } from '@oxide/api' +import { useApi } from '@oxide/api' import { GlobalNav, OperationList, ProjectList } from '@oxide/ui' import Wordmark from '../assets/wordmark.svg' @@ -56,7 +56,7 @@ const GlobalNavContainer = styled.header` ` export default ({ children }: AppLayoutProps) => { - const { data: projects } = useApiData(api.apiProjectsGet, {}) + const { data: projects } = useApi('apiProjectsGet', {}) return ( diff --git a/apps/web-console/src/pages/instance/InstancePage.tsx b/apps/web-console/src/pages/instance/InstancePage.tsx index 90ab8e6405..7cb1194f12 100644 --- a/apps/web-console/src/pages/instance/InstancePage.tsx +++ b/apps/web-console/src/pages/instance/InstancePage.tsx @@ -2,7 +2,7 @@ import React from 'react' import { useParams } from 'react-router-dom' import styled from 'styled-components' -import { useApiData, api } from '@oxide/api' +import { useApi } from '@oxide/api' import { Breadcrumbs, @@ -74,7 +74,7 @@ const InstancePage = () => { const breadcrumbs = useBreadcrumbs() const { projectName, instanceName } = useParams() - const { data, error } = useApiData(api.apiProjectInstancesGetInstance, { + const { data, error } = useApi('apiProjectInstancesGetInstance', { instanceName, projectName, }) diff --git a/apps/web-console/src/pages/instance/InstancesPage.tsx b/apps/web-console/src/pages/instance/InstancesPage.tsx index f5b4272ca5..de3df2efc1 100644 --- a/apps/web-console/src/pages/instance/InstancesPage.tsx +++ b/apps/web-console/src/pages/instance/InstancesPage.tsx @@ -3,7 +3,7 @@ import styled from 'styled-components' import { useParams, Link } from 'react-router-dom' -import { useApiData, api } from '@oxide/api' +import { useApi } from '@oxide/api' import { Breadcrumbs, PageHeader, TextWithIcon } from '@oxide/ui' import { useBreadcrumbs } from '../../hooks' @@ -20,7 +20,7 @@ const InstancesPage = () => { const breadcrumbs = useBreadcrumbs() const { projectName } = useParams() - const { data } = useApiData(api.apiProjectInstancesGet, { projectName }) + const { data } = useApi('apiProjectInstancesGet', { projectName }) if (!data) return
loading
diff --git a/apps/web-console/src/pages/projects/ProjectPage.tsx b/apps/web-console/src/pages/projects/ProjectPage.tsx index b5c36c69a4..55274781ed 100644 --- a/apps/web-console/src/pages/projects/ProjectPage.tsx +++ b/apps/web-console/src/pages/projects/ProjectPage.tsx @@ -3,7 +3,7 @@ import styled from 'styled-components' import { useParams, Link } from 'react-router-dom' -import { useApiData, api } from '@oxide/api' +import { useApi } from '@oxide/api' import { Breadcrumbs, PageHeader, TextWithIcon } from '@oxide/ui' import { useBreadcrumbs } from '../../hooks' @@ -20,10 +20,10 @@ const ProjectPage = () => { const breadcrumbs = useBreadcrumbs() const { projectName } = useParams() - const { data: project } = useApiData(api.apiProjectsGetProject, { + const { data: project } = useApi('apiProjectsGetProject', { projectName, }) - const { data: instances } = useApiData(api.apiProjectInstancesGet, { + const { data: instances } = useApi('apiProjectInstancesGet', { projectName, }) diff --git a/apps/web-console/src/pages/projects/ProjectsPage.tsx b/apps/web-console/src/pages/projects/ProjectsPage.tsx index b2fe8de968..04ebfe12ed 100644 --- a/apps/web-console/src/pages/projects/ProjectsPage.tsx +++ b/apps/web-console/src/pages/projects/ProjectsPage.tsx @@ -3,7 +3,7 @@ import styled from 'styled-components' import { Link } from 'react-router-dom' -import { useApiData, api } from '@oxide/api' +import { useApi } from '@oxide/api' import { useBreadcrumbs } from '../../hooks' import { Breadcrumbs, PageHeader, TextWithIcon } from '@oxide/ui' @@ -14,7 +14,7 @@ const Title = styled(TextWithIcon).attrs({ const ProjectsPage = () => { const breadcrumbs = useBreadcrumbs() - const { data } = useApiData(api.apiProjectsGet, {}) + const { data } = useApi('apiProjectsGet', {}) if (!data) return
loading
diff --git a/libs/api/hooks/index.ts b/libs/api/hooks/index.ts index 0de75646a1..aa4bc75865 100644 --- a/libs/api/hooks/index.ts +++ b/libs/api/hooks/index.ts @@ -1,36 +1 @@ -import useSWR from 'swr' - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type Params = Record - -// TODO: write tests for this -export const sortObj = (obj: Params): Params => { - const sorted: Params = {} - for (const k of Object.keys(obj).sort()) { - sorted[k] = obj[k] - } - return sorted -} - -// The first argument to useSWR in the standard use case would be -// the URL to fetch. It is used to uniquely identify the request -// for caching purposes. If multiple components request the same -// thing at the same time, SWR will only make one HTTP request. -// Because we have a generated client library, we do not have URLs. -// Instead, we have function names and parameter objects. But object -// literals do not have referential stability across renders, so we -// have to use JSON.stringify to turn the params into a stable key. -// We also sort the keys in the params object so that { a: 1, b: 2 } -// and { b: 2, a: 1 } are considered equivalent. SWR accepts an array -// of strings as well as a single string. -export function useApiData

( - method: (p: P) => Promise, - params: P -) { - if (process.env.NODE_ENV === 'development' && method.name === '') { - throw new Error('API method must have a name') - } - - const paramsStr = JSON.stringify(sortObj(params)) - return useSWR([method.name, paramsStr], () => method(params)) -} +export { getUseApi } from './use-api-data' diff --git a/libs/api/hooks/index.spec.ts b/libs/api/hooks/use-api-data.spec.ts similarity index 92% rename from libs/api/hooks/index.spec.ts rename to libs/api/hooks/use-api-data.spec.ts index 2c140f8b9f..2f5b2a1ac4 100644 --- a/libs/api/hooks/index.spec.ts +++ b/libs/api/hooks/use-api-data.spec.ts @@ -1,4 +1,4 @@ -import { sortObj } from './index' +import { sortObj } from './use-api-data' describe('sortObj', () => { it('sorts object keys alphabetically', () => { diff --git a/libs/api/hooks/use-api-data.ts b/libs/api/hooks/use-api-data.ts new file mode 100644 index 0000000000..318bcbbe8c --- /dev/null +++ b/libs/api/hooks/use-api-data.ts @@ -0,0 +1,79 @@ +import useSWR from 'swr' + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Params = Record + +export const sortObj = (obj: Params): Params => { + const sorted: Params = {} + for (const k of Object.keys(obj).sort()) { + sorted[k] = obj[k] + } + return sorted +} + +// https://github.com/piotrwitek/utility-types/tree/df2502e#pickbyvaluet-valuetype +type PickByValue = Pick< + T, + { [Key in keyof T]-?: T[Key] extends ValueType ? Key : never }[keyof T] +> + +/* eslint-disable @typescript-eslint/no-explicit-any */ + +// given an API object A and a key K where A[K] is a function that takes a +// single argument and returns a promise... + +// extract the type of the argument (if it extends Params) +type ReqParams = A[K] extends (p: infer P) => Promise + ? P extends Params + ? P + : never + : never + +// extract the type of the value inside the promise +type Response = A[K] extends (p: any) => Promise + ? R + : never + +// This all needs explanation. The easiest starting point is what this would +// look like in plain JS, which is quite simple: +// +// const getUseApi = (api) => (method, params) => { +// const paramsStr = JSON.stringify(sortObj(params)) +// return useSWR([method, paramsStr], () => api[method](params)) +// } +// +// 1. what's up with the JSON.stringify/ +// +// The first argument to useSWR in the standard use case would be the URL to +// fetch. It is used to uniquely identify the request for caching purposes. If +// multiple components request the same thing at the same time, SWR will only +// make one HTTP request. Because we have a generated client library, we do not +// have URLs. Instead, we have function names and parameter objects. But object +// literals do not have referential stability across renders, so we have to use +// JSON.stringify to turn the params into a stable key. We also sort the keys in +// the params object so that { a: 1, b: 2 } and { b: 2, a: 1 } are considered +// equivalent. SWR accepts an array of strings as well as a single string. +// +// 2. what's up with the types? +// +// The type situation here is pretty gnarly considering how simple the plain JS +// version is. The difficulty is that we want full type safety, i.e., based on +// the method name passed in, we want the typechecker to check the params and +// annotate the response. PickByValue ensures we only call methods on the API +// object that follow the (params) => Promise pattern. Then we use the +// inferred type of the key (the method name) to enforce that params match the +// expected params on the named method. Finally we use the Response helper to +// tell useSWR what type to put on the response data. +export function getUseApi Promise>>( + api: A +) { + function useApi(method: K, params: ReqParams) { + const paramsStr = JSON.stringify(sortObj(params)) + return useSWR>([method, paramsStr], () => + api[method](params) + ) + } + return useApi +} + +/* eslint-enable @typescript-eslint/no-explicit-any */ diff --git a/libs/api/index.ts b/libs/api/index.ts index fb12cf0451..17e2757ed6 100644 --- a/libs/api/index.ts +++ b/libs/api/index.ts @@ -1,5 +1,4 @@ -export * from './__generated__' -export * from './hooks' +import { getUseApi } from './hooks' import { DefaultApi, Configuration } from './__generated__' @@ -8,16 +7,7 @@ const config = ? new Configuration({ basePath: process.env.API_URL }) : new Configuration({ basePath: '/api' }) -export const api = new DefaultApi(config) +const api = new DefaultApi(config) -// the API methods rely on `this` being bound to the API object. in order to -// pass the methods around as arguments without explicitly calling .bind(this) -// every time, we just bind them all right here. TS doesn't like this, so we throw -// an `any` in there, but it's all above board. -Object.getOwnPropertyNames(DefaultApi.prototype) - .filter((prop) => prop.startsWith('api')) - .forEach((prop) => { - const key = prop as keyof DefaultApi - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ;(api as any)[key] = api[key].bind(api) - }) +export const useApi = getUseApi(api) +export * from './__generated__' diff --git a/package.json b/package.json index abf15a9305..0286f94d3e 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,6 @@ "@types/react-virtualized-auto-sizer": "^1.0.0", "@types/react-window": "^1.8.2", "@types/styled-components": "5.1.4", - "@types/terser-webpack-plugin": "^5.0.3", "@types/uuid": "^8.3.0", "@types/webpack": "^4.41.26", "@types/webpack-dev-server": "^3.11.2", diff --git a/webpack.prod.config.ts b/webpack.prod.config.ts index 692b0fcae8..d6bf28044c 100644 --- a/webpack.prod.config.ts +++ b/webpack.prod.config.ts @@ -1,7 +1,6 @@ import path from 'path' import webpack from 'webpack' import { CleanWebpackPlugin } from 'clean-webpack-plugin' -import TerserPlugin from 'terser-webpack-plugin' import sharedConfig from './webpack.shared.config' const config = { @@ -19,18 +18,6 @@ const config = { }), new CleanWebpackPlugin(), ], - optimization: { - minimize: true, - minimizer: [ - new TerserPlugin({ - terserOptions: { - // HACK: temporary fix to make API hook relying on function name work - // in production. remove to decrease bundle size once hook is improved - keep_fnames: true, - }, - }), - ], - }, } export default config diff --git a/yarn.lock b/yarn.lock index 3a7806eb0a..06694d4c92 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3405,14 +3405,6 @@ resolved "https://registry.yarnpkg.com/@types/tapable/-/tapable-1.0.7.tgz#545158342f949e8fd3bfd813224971ecddc3fac4" integrity sha512-0VBprVqfgFD7Ehb2vd8Lh9TG3jP98gvr8rgehQqzztZNI7o8zS8Ad4jyZneKELphpuE212D8J70LnSNQSyO6bQ== -"@types/terser-webpack-plugin@^5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@types/terser-webpack-plugin/-/terser-webpack-plugin-5.0.3.tgz#9194c24dee3a9d5dcfd67b58edffc1d66653d16b" - integrity sha512-Ef60BOY9hV+yXjkMCuJI17cu1R8/H31n5Rnt1cElJFyBSkbRV3UWyBIYn8YpijsOG05R4bZf3G2azyBHkksu/A== - dependencies: - terser "^5.3.8" - webpack "^5.1.0" - "@types/through@*": version "0.0.30" resolved "https://registry.yarnpkg.com/@types/through/-/through-0.0.30.tgz#e0e42ce77e897bd6aead6f6ea62aeb135b8a3895" @@ -13598,15 +13590,6 @@ terser@^4.1.2, terser@^4.6.3, terser@^4.8.0: source-map "~0.6.1" source-map-support "~0.5.12" -terser@^5.3.8: - version "5.6.1" - resolved "https://registry.yarnpkg.com/terser/-/terser-5.6.1.tgz#a48eeac5300c0a09b36854bf90d9c26fb201973c" - integrity sha512-yv9YLFQQ+3ZqgWCUk+pvNJwgUTdlIxUk1WTN+RnaFJe2L7ipG2csPT0ra2XRm7Cs8cxN7QXmK1rFzEwYEQkzXw== - dependencies: - commander "^2.20.0" - source-map "~0.7.2" - source-map-support "~0.5.19" - terser@^5.5.1: version "5.5.1" resolved "https://registry.yarnpkg.com/terser/-/terser-5.5.1.tgz#540caa25139d6f496fdea056e414284886fb2289" @@ -14626,35 +14609,6 @@ webpack@4: watchpack "^1.7.4" webpack-sources "^1.4.1" -webpack@^5.1.0: - version "5.30.0" - resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.30.0.tgz#07d87c182a060e0c2491062f3dc0edc85a29d884" - integrity sha512-Zr9NIri5yzpfmaMea2lSMV1UygbW0zQsSlGLMgKUm63ACXg6alhd1u4v5UBSBjzYKXJN6BNMGVM7w165e7NxYA== - dependencies: - "@types/eslint-scope" "^3.7.0" - "@types/estree" "^0.0.46" - "@webassemblyjs/ast" "1.11.0" - "@webassemblyjs/wasm-edit" "1.11.0" - "@webassemblyjs/wasm-parser" "1.11.0" - acorn "^8.0.4" - browserslist "^4.14.5" - chrome-trace-event "^1.0.2" - enhanced-resolve "^5.7.0" - es-module-lexer "^0.4.0" - eslint-scope "^5.1.1" - events "^3.2.0" - glob-to-regexp "^0.4.1" - graceful-fs "^4.2.4" - json-parse-better-errors "^1.0.2" - loader-runner "^4.2.0" - mime-types "^2.1.27" - neo-async "^2.6.2" - schema-utils "^3.0.0" - tapable "^2.1.1" - terser-webpack-plugin "^5.1.1" - watchpack "^2.0.0" - webpack-sources "^2.1.1" - webpack@^5.26.0: version "5.26.0" resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.26.0.tgz#269841ed7b5c6522221aa27f7efceda04c8916d7"