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

refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

Merged
merged 45 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
ef8432b
initial commit: wire new router
abailly-akamai Oct 24, 2024
1825a08
Migrate test to use new helper
abailly-akamai Oct 24, 2024
1f76c3b
volumes landing routing
abailly-akamai Oct 24, 2024
906d1a1
doin the filtering
abailly-akamai Oct 25, 2024
9cd1480
wire actions
abailly-akamai Oct 25, 2024
ec44117
cleanup
abailly-akamai Oct 25, 2024
b5a73d4
cleanup
abailly-akamai Oct 25, 2024
3770439
moar cleanup
abailly-akamai Oct 25, 2024
7faf676
more work on params
abailly-akamai Oct 25, 2024
5873b3c
usePaginationV2
abailly-akamai Oct 28, 2024
4fe2d4f
cleanup
abailly-akamai Oct 28, 2024
e2c3a08
oops fix types
abailly-akamai Oct 28, 2024
2ab5a69
fix e2e
abailly-akamai Oct 28, 2024
80bf0d6
useOrderV2
abailly-akamai Oct 28, 2024
c563c83
and... dont forget the util 🀦
abailly-akamai Oct 28, 2024
c5258a8
revert unwarranted change
abailly-akamai Oct 28, 2024
77490ce
useOrderV2 test
abailly-akamai Oct 29, 2024
9be5e51
console error
abailly-akamai Oct 29, 2024
bdfa4dd
A bit more cleanup
abailly-akamai Oct 29, 2024
af33dbf
usePaginationV2 test
abailly-akamai Oct 29, 2024
e0d9566
usePaginationV2 test
abailly-akamai Oct 29, 2024
5656d01
route level error handling
abailly-akamai Oct 30, 2024
9062419
xFilter util
abailly-akamai Oct 30, 2024
403ea72
xFilter util improvements
abailly-akamai Oct 30, 2024
49f296e
post rebase fix
abailly-akamai Oct 31, 2024
55f909a
testing xQuery builder
abailly-akamai Oct 31, 2024
748eb0d
moar testing and cleanup
abailly-akamai Oct 31, 2024
8696686
Added changeset: Migrate `/volumes` to Tanstack router
abailly-akamai Oct 31, 2024
5337929
Save progress
abailly-akamai Nov 4, 2024
30938ae
fix one test
abailly-akamai Nov 5, 2024
22ca472
fix remaining test
abailly-akamai Nov 6, 2024
3ed5e99
feedback @jaalah-akamai
abailly-akamai Nov 6, 2024
ac19483
more work on preloading
abailly-akamai Nov 6, 2024
6c5c570
save progress
abailly-akamai Nov 6, 2024
8af2e9a
Walking back and using a more progressive approach
abailly-akamai Nov 8, 2024
92ace27
cleanup
abailly-akamai Nov 8, 2024
a3de9ca
entity not found logic
abailly-akamai Nov 8, 2024
4a39156
post rebase fix
abailly-akamai Nov 15, 2024
6927d90
post rebase fix
abailly-akamai Nov 20, 2024
cc2198e
update loading state
abailly-akamai Nov 26, 2024
2d228e2
fix the smoke tests
abailly-akamai Nov 27, 2024
83103bd
Feedback @bnussman-akamai
abailly-akamai Dec 2, 2024
0c09de5
JsDocs for Tanstack link components
abailly-akamai Dec 2, 2024
0b0d678
Improve new useOrder hook
abailly-akamai Dec 10, 2024
afaaa1d
feedback @coliu-akamai @hkhalil-akamai
abailly-akamai Dec 10, 2024
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Migrate `/volumes` to Tanstack router ([#11154](https://github.com/linode/manager/pull/11154))
6 changes: 5 additions & 1 deletion packages/manager/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,14 @@ module.exports = {
{
files: [
// for each new features added to the migration router, add its directory here
'src/features/Betas/*',
'src/features/Betas/**/*',
'src/features/Volumes/**/*',
],
rules: {
'no-restricted-imports': [
// This needs to remain an error however trying to link to a feature that is not yet migrated will break the router
// For those cases react-router-dom history.push is still needed
// using `eslint-disable-next-line no-restricted-imports` can help bypass those imports
'error',
{
paths: [
Expand Down
13 changes: 10 additions & 3 deletions packages/manager/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import {
Controls,
Stories,
} from '@storybook/blocks';
import { wrapWithTheme } from '../src/utilities/testHelpers';
import {
wrapWithTheme,
wrapWithThemeAndRouter,
} from '../src/utilities/testHelpers';
import { useDarkMode } from 'storybook-dark-mode';
import { DocsContainer as BaseContainer } from '@storybook/addon-docs';
import { themes } from '@storybook/theming';
Expand Down Expand Up @@ -42,9 +45,13 @@ export const DocsContainer = ({ children, context }) => {

const preview: Preview = {
decorators: [
(Story) => {
(Story, context) => {
const isDark = useDarkMode();
return wrapWithTheme(<Story />, { theme: isDark ? 'dark' : 'light' });
return context.parameters.tanStackRouter
? wrapWithThemeAndRouter(<Story />, {
theme: isDark ? 'dark' : 'light',
})
: wrapWithTheme(<Story />, { theme: isDark ? 'dark' : 'light' });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this allows routed components to render properly - should not be needed often

},
],
loaders: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from 'support/intercepts/linodes';
import {
mockCreateVolume,
mockGetVolume,
mockGetVolumes,
mockDetachVolume,
mockGetVolumeTypesError,
Expand Down Expand Up @@ -85,6 +86,7 @@ describe('volumes', () => {

mockGetVolumes([]).as('getVolumes');
mockCreateVolume(mockVolume).as('createVolume');
mockGetVolume(mockVolume).as('getVolume');
mockGetVolumeTypes(mockVolumeTypes).as('getVolumeTypes');

cy.visitWithLogin('/volumes', {
Expand Down Expand Up @@ -114,7 +116,7 @@ describe('volumes', () => {

mockGetVolumes([mockVolume]).as('getVolumes');
ui.button.findByTitle('Create Volume').should('be.visible').click();
cy.wait(['@createVolume', '@getVolumes']);
cy.wait(['@createVolume', '@getVolume', '@getVolumes']);
validateBasicVolume(mockVolume.label);

ui.actionMenu
Expand Down Expand Up @@ -193,6 +195,7 @@ describe('volumes', () => {

mockDetachVolume(mockAttachedVolume.id).as('detachVolume');
mockGetVolumes([mockAttachedVolume]).as('getAttachedVolumes');
mockGetVolume(mockAttachedVolume).as('getVolume');
cy.visitWithLogin('/volumes', {
preferenceOverrides,
localStorageOverrides,
Expand All @@ -209,6 +212,8 @@ describe('volumes', () => {

ui.actionMenuItem.findByTitle('Detach').click();

cy.wait('@getVolume');

ui.dialog
.findByTitle(`Detach Volume ${mockAttachedVolume.label}?`)
.should('be.visible')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import {
mockGetLinodeDisks,
mockGetLinodeVolumes,
} from 'support/intercepts/linodes';
import { mockMigrateVolumes, mockGetVolumes } from 'support/intercepts/volumes';
import {
mockMigrateVolumes,
mockGetVolumes,
mockGetVolume,
} from 'support/intercepts/volumes';
import { ui } from 'support/ui';

describe('volume upgrade/migration', () => {
Expand All @@ -23,6 +27,7 @@ describe('volume upgrade/migration', () => {
});

mockGetVolumes([volume]).as('getVolumes');
mockGetVolume(volume).as('getVolume');
mockMigrateVolumes().as('migrateVolumes');
mockGetNotifications([migrationScheduledNotification]).as(
'getNotifications'
Expand Down Expand Up @@ -53,7 +58,7 @@ describe('volume upgrade/migration', () => {
.click();
});

cy.wait(['@migrateVolumes', '@getNotifications']);
cy.wait(['@migrateVolumes', '@getVolume', '@getNotifications']);

cy.findByText('UPGRADE PENDING').should('be.visible');

Expand Down
6 changes: 3 additions & 3 deletions packages/manager/src/MainContent.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Box } from '@linode/ui';
import Grid from '@mui/material/Unstable_Grid2';
import { useQueryClient } from '@tanstack/react-query';
import { RouterProvider } from '@tanstack/react-router';
import * as React from 'react';
import { Redirect, Route, Switch } from 'react-router-dom';
Expand Down Expand Up @@ -130,7 +131,6 @@ const LinodesRoutes = React.lazy(() =>
default: module.LinodesRoutes,
}))
);
const Volumes = React.lazy(() => import('src/features/Volumes'));
const Domains = React.lazy(() =>
import('src/features/Domains').then((module) => ({
default: module.DomainsRoutes,
Expand Down Expand Up @@ -207,6 +207,7 @@ export const MainContent = () => {
const { classes, cx } = useStyles();
const { data: preferences } = usePreferences();
const { mutateAsync: updatePreferences } = useMutatePreferences();
const queryClient = useQueryClient();

const globalErrors = useGlobalErrors();

Expand Down Expand Up @@ -336,8 +337,6 @@ export const MainContent = () => {
path="/placement-groups"
/>
)}
<Route component={Volumes} path="/volumes" />
<Redirect path="/volumes*" to="/volumes" />
<Route
component={NodeBalancers}
path="/nodebalancers"
Expand Down Expand Up @@ -382,6 +381,7 @@ export const MainContent = () => {
*/}
<Route path="*">
<RouterProvider
context={{ queryClient }}
router={migrationRouter as AnyRouter}
/>
</Route>
Expand Down
2 changes: 2 additions & 0 deletions packages/manager/src/Router.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { QueryClient } from '@tanstack/react-query';
import { RouterProvider } from '@tanstack/react-router';
import * as React from 'react';

Expand All @@ -24,6 +25,7 @@ export const Router = () => {
isACLPEnabled,
isDatabasesEnabled,
isPlacementGroupsEnabled,
queryClient: new QueryClient(),
},
});

Expand Down
38 changes: 38 additions & 0 deletions packages/manager/src/components/TanstackLink.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import React from 'react';

import { TanstackLink } from './TanstackLinks';

import type { TanstackLinkComponentProps } from './TanstackLinks';
import type { Meta, StoryObj } from '@storybook/react';

export const AsButtonPrimary: StoryObj<TanstackLinkComponentProps> = {
render: () => (
<TanstackLink linkType="primary" to="/">
Home
</TanstackLink>
),
};

export const AsButtonSecondary: StoryObj<TanstackLinkComponentProps> = {
render: () => (
<TanstackLink linkType="secondary" to="/">
Home
</TanstackLink>
),
};

export const AsLink: StoryObj<TanstackLinkComponentProps> = {
render: () => (
<TanstackLink linkType="link" to="/">
Home
</TanstackLink>
),
};

const meta: Meta<TanstackLinkComponentProps> = {
parameters: {
tanStackRouter: true,
},
title: 'Foundations/Link/Tanstack Link',
};
export default meta;
83 changes: 83 additions & 0 deletions packages/manager/src/components/TanstackLinks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { Button } from '@linode/ui';
import { omitProps } from '@linode/ui';
import { LinkComponent } from '@tanstack/react-router';
import { createLink } from '@tanstack/react-router';
import * as React from 'react';

import { MenuItem } from 'src/components/MenuItem';

import type { ButtonProps, ButtonType } from '@linode/ui';
import type { LinkProps as TanStackLinkProps } from '@tanstack/react-router';

export interface TanstackLinkComponentProps
extends Omit<ButtonProps, 'buttonType' | 'href'> {
linkType: 'link' | ButtonType;
tooltipAnalyticsEvent?: (() => void) | undefined;
tooltipText?: string;
}

export interface TanStackLinkRoutingProps {
linkType: TanstackLinkComponentProps['linkType'];
params?: TanStackLinkProps['params'];
preload?: TanStackLinkProps['preload'];
search?: TanStackLinkProps['search'];
to: TanStackLinkProps['to'];
}

const LinkComponent = React.forwardRef<
HTMLButtonElement,
TanstackLinkComponentProps
>((props, ref) => {
const _props = omitProps(props, ['linkType']);

return <Button component="a" ref={ref} {..._props} />;
abailly-akamai marked this conversation as resolved.
Show resolved Hide resolved
});

const MenuItemLinkComponent = React.forwardRef<
HTMLButtonElement,
TanstackLinkComponentProps
>((props, ref) => {
const _props = omitProps(props, ['linkType']);

return <MenuItem component="a" ref={ref} {..._props} />;
});

const CreatedLinkComponent = createLink(LinkComponent);
const CreatedMenuItemLinkComponent = createLink(MenuItemLinkComponent);

/**
* @deprecated
* This is marked as deprecated to discourage usage until the migration is complete.
* While not technically deprecated, these components should not be used anywhere in the app.
* They will be replacing our Links eventually, but have only been introduced early to test their functionality.
*/
export const TanstackLink: LinkComponent<typeof LinkComponent> = (props) => {
return (
<CreatedLinkComponent
{...props}
sx={(theme) => ({
...(props.linkType === 'link' && {
'&:hover': {
textDecoration: 'underline',
},
fontFamily: theme.font.normal,
fontSize: '0.875rem',
minWidth: 'initial',
padding: 0,
}),
})}
/>
);
};

/**
* @deprecated
* This is marked as deprecated to discourage usage until the migration is complete.
* While not technically deprecated, these components should not be used anywhere in the app.
* They will be replacing our Links eventually, but have only been introduced early to test their functionality.
*/
export const TanstackMenuItemLink: LinkComponent<
typeof MenuItemLinkComponent
> = (props) => {
return <CreatedMenuItemLinkComponent {...props} />;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this component is not in use in this PR - it was introduced while exploring prefetching, which is too early to do during the migration of some feature. I left it in here because it will be helpful in the near future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coliu-akamai to answer your question above, both components in this file are marked as @deprecated ( i wish there was a supported @experimental flag)

Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export const TypeToConfirmDialog = (props: CombinedProps) => {
entity,
errors,
inputProps,
isFetching,
label,
loading,
onClick,
Expand Down Expand Up @@ -131,6 +132,7 @@ export const TypeToConfirmDialog = (props: CombinedProps) => {
<ConfirmationDialog
actions={actions}
error={errors ? errors[0].reason : undefined}
isFetching={isFetching}
onClose={onClose}
open={open}
title={title}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react';

import { accountFactory, volumeFactory } from 'src/factories';
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { renderWithTheme } from 'src/utilities/testHelpers';
import { renderWithThemeAndRouter } from 'src/utilities/testHelpers';

import { AttachVolumeDrawer } from './AttachVolumeDrawer';

Expand All @@ -24,7 +24,7 @@ describe('AttachVolumeDrawer', () => {
})
);

const { getByLabelText } = renderWithTheme(
const { getByLabelText } = await renderWithThemeAndRouter(
<AttachVolumeDrawer onClose={vi.fn} open volume={volume} />,
{
flags: { blockStorageEncryption: true },
Expand All @@ -48,7 +48,7 @@ describe('AttachVolumeDrawer', () => {
})
);

const { queryByRole } = renderWithTheme(
const { queryByRole } = await renderWithThemeAndRouter(
<AttachVolumeDrawer onClose={vi.fn} open volume={volume} />,
{
flags: { blockStorageEncryption: false },
Expand All @@ -69,7 +69,7 @@ describe('AttachVolumeDrawer', () => {
})
);

const { queryByRole } = renderWithTheme(
const { queryByRole } = await renderWithThemeAndRouter(
<AttachVolumeDrawer onClose={vi.fn} open volume={volume} />,
{
flags: { blockStorageEncryption: true },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ConfigSelect } from './VolumeDrawer/ConfigSelect';
import type { Volume } from '@linode/api-v4';

interface Props {
isFetching?: boolean;
onClose: () => void;
open: boolean;
volume: Volume | undefined;
Expand All @@ -35,7 +36,7 @@ const AttachVolumeValidationSchema = object({
});

export const AttachVolumeDrawer = React.memo((props: Props) => {
const { open, volume } = props;
const { isFetching, open, volume } = props;

const { enqueueSnackbar } = useSnackbar();

Expand Down Expand Up @@ -97,6 +98,7 @@ export const AttachVolumeDrawer = React.memo((props: Props) => {

return (
<Drawer
isFetching={isFetching}
onClose={handleClose}
open={open}
title={`Attach Volume ${volume?.label}`}
Expand Down
Loading
Loading