chore(Coder plugin): update all Backstage code to use preview SDK #131
Conversation
| @@ -100,50 +96,6 @@ describe(`${CoderClient.name}`, () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('cleanupClient functionality', () => { | |||
There was a problem hiding this comment.
Deleted because the cleanup method no longer exists
| const assetsEndpoint = await urlSync.getAssetsEndpoint(); | ||
|
|
||
| const allWorkspacesAreRemapped = !workspaces.some(ws => | ||
| ws.template_icon.startsWith(apiEndpoint), | ||
| const allWorkspacesAreRemapped = workspaces.every(ws => | ||
| ws.template_icon.startsWith(assetsEndpoint), |
There was a problem hiding this comment.
Typo – didn't realize when I wrote the tests that I was using the wrong endpoint type
| type RequestInterceptor = ( | ||
| config: RequestConfig, | ||
| ) => RequestConfig | Promise<RequestConfig>; |
There was a problem hiding this comment.
Extracted this out to make addRequestInterceptor have slightly less wonky Prettier formatting
| const registerNewToken = useCallback((newToken: string) => { | ||
| if (newToken !== '') { | ||
| setAuthToken(newToken); | ||
| } | ||
| }, []); | ||
|
|
||
| const ejectToken = useCallback(() => { | ||
| setAuthToken(''); | ||
| window.localStorage.removeItem(TOKEN_STORAGE_KEY); | ||
| queryClient.removeQueries({ queryKey: [CODER_QUERY_KEY_PREFIX] }); | ||
| }, [queryClient]); |
There was a problem hiding this comment.
It's not a big deal for our use cases, but just because these functions are going to exported to end-users, I though it'd be good to stabilize their memory references, so that people run into fewer useEffect pain points
| @@ -100,50 +96,6 @@ describe(`${CoderClient.name}`, () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('cleanupClient functionality', () => { | |||
| axiosInstance: AxiosInstance, | ||
| ): BackstageCoderSdk { | ||
| const baseSdk = new CoderSdk(axiosInstance); | ||
| private getBackstageCoderSdk(): BackstageCoderSdk { |
There was a problem hiding this comment.
Tiny nit especially considering this is private, but should we call this makeBackstageCoderSdk? I think get can give off singleton vibes.
There was a problem hiding this comment.
Yeah, good call. I think I've always had a habit of using get for anything that produces a value, but I've been realizing lately that make and create are usually more explicit
| @@ -48,7 +48,7 @@ type UrlPrefixes = Readonly<{ | |||
|
|
|||
| export const defaultUrlPrefixes = { | |||
| proxyPrefix: `/api/proxy`, | |||
| apiRoutePrefix: '/api/v2', | |||
| apiRoutePrefix: '', // Left as empty string because code assumes that CoderSdk will add /api/v2 | |||
There was a problem hiding this comment.
Can we delete this if it is always blank? Or at least, from my brief searching I did not find any instances of apiRoutePrefix being set to something else. Same with assetsRoutePrefix. I wonder if I just missed something though.
There was a problem hiding this comment.
No, you're not missing anything. I guess I decided to keep the property because I thought that maybe down the line, it might make sense to let the prefix be dynamic
Even if we do need that later, though, it should be easy to add back later. I'll get rid of this
Closes #114
Changes made
CoderClientcleanupmethod, since it isn't necessary for testing anymore – the SDK itself is easy to instantiate per test case and doesn't require explicit cleanupuseCoderSdkto forward a few more properties that would be helpful to end-users