From f4fffa3607c3f8e8d2f09553d1ebf136b539fc17 Mon Sep 17 00:00:00 2001 From: Anthony Date: Mon, 21 Aug 2023 15:13:34 +0800 Subject: [PATCH] [EVNT-59] Workaround for typescript issue https://github.com/microsoft/TypeScript/issues/48212 --- DEVELOPMENT.md | 49 ++++++++++++++++++++ src/components/NavBar/Styling.tsx | 13 ++++-- src/components/NavBar/index.stories.tsx | 4 +- src/layouts/AppLayout/NavBar/Styling.tsx | 12 +++-- src/layouts/PageLayout/PageContainer.tsx | 6 ++- src/layouts/PageLayout/PanelAwareMargins.tsx | 9 +++- src/layouts/PageLayout/index.stories.tsx | 4 +- 7 files changed, 85 insertions(+), 12 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 6ab9b18..fc43aec 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -15,6 +15,55 @@ this. However at the time it was not a priority. Also, each component will likely be relatively stand alone, so using relative paths for local imports unlikely to be arduous, at least in the near term. +## Regarding typing of thing like "Root = styled(...)" + +At the time of writing, with typescript 5.0.2, this project is affected by a +typescript issue involved nested package types, which in particular crops up +when uses MUI's `styled()` utility. + +For example you have the following component defined as: + +```ts +import { styled} from '@mui/system'; +import { Box } from '@mui/material'; + +export const Root = styled(Box, { + name: 'NavBar', +})(({ theme }) => ({ + // ... object containing styling +})); +``` + +You'll get a typescrypt error along the lines of: + +```text +src/components/NavBar/Test.tsx(4,14): error TS2742: The inferred type of +'Root' cannot be named without a reference to +'.pnpm/@mui+system@5.14.5_@emotion+react@11.11.1_@emotion+styled@11.11.0_ +@types+react@18.2.0_react@18.2.0/node_modules/@mui/system'. This is likely not +portable. A type annotation is necessary. +``` + +This is currently an open issue: +, related to nested +modules. It is currently milestoned to be resolved in 5.1.0, but that was +already pushed out from 4.8.0 + +There are many workarounds, but the one we landed on was to use a type +annotation as the warning says. The component would then be defind as: + +```ts +import { styled } from '@mui/material/styles'; // Not @mui/system +import { Box, BoxProps } from '@mui/material'; +import { StyledComponent } from '@emotion/styled'; + +export const Root:StyledComponent = styled(Box, { + name: 'NavBar', +})(({ theme }) => ({ + // ... object containing styling +})); +``` + ## Yarn/npm linking TODO: Currently experimenting with [pnpm](https://pnpm.io) so this is out of diff --git a/src/components/NavBar/Styling.tsx b/src/components/NavBar/Styling.tsx index be0a9aa..848365f 100644 --- a/src/components/NavBar/Styling.tsx +++ b/src/components/NavBar/Styling.tsx @@ -1,5 +1,6 @@ import { styled, CSSObject } from '@mui/material/styles'; -import Drawer from '@mui/material/Drawer'; +import { StyledComponent } from '@emotion/styled'; +import { Box, BoxProps, Drawer, DrawerProps } from '@mui/material'; export const NAVBAR_WIDTH_OPENED = 330; export const NAVBAR_WIDTH_CLOSED = 73; @@ -16,7 +17,11 @@ export const classes = { pieChartIcon: `${PREFIX}-pieChartIcon`, }; -export const Root = styled('div', { name: 'NavBar' })(({ theme }) => ({ +// TODO: Explicit type annotation needed until following issue fixed: +// https://github.com/microsoft/TypeScript/issues/48212 +export const Root: StyledComponent = styled(Box, { + name: 'NavBar', +})(({ theme }) => ({ [`&.${classes.root}`]: { display: 'flex', }, @@ -64,7 +69,9 @@ const closedMixin = (): CSSObject => ({ ...sharedOverrides(), }); -export const NavDrawer = styled(Drawer, { +// TODO: Explicit type annotation needed until following issue fixed: +// https://github.com/microsoft/TypeScript/issues/48212 +export const NavDrawer: StyledComponent = styled(Drawer, { shouldForwardProp: (prop) => prop !== 'open', })(({ theme, open }) => ({ width: open ? NAVBAR_WIDTH_OPENED : NAVBAR_WIDTH_CLOSED, diff --git a/src/components/NavBar/index.stories.tsx b/src/components/NavBar/index.stories.tsx index 9d86d1a..5784cfe 100644 --- a/src/components/NavBar/index.stories.tsx +++ b/src/components/NavBar/index.stories.tsx @@ -11,7 +11,9 @@ import { Title, Description, Primary, Controls, Stories } from '@storybook/block import NavBar, { NavBarProvider, NavBarDarkStyledList } from '.'; -const meta = { +// TODO: It shouldn't be encessary to have this explicit type annotation here. +// It is a workaround for https://github.com/microsoft/TypeScript/issues/48212 +const meta: Meta = { component: NavBar, tags: ['autodocs'], decorators: [ diff --git a/src/layouts/AppLayout/NavBar/Styling.tsx b/src/layouts/AppLayout/NavBar/Styling.tsx index 8a2d6ec..875f885 100644 --- a/src/layouts/AppLayout/NavBar/Styling.tsx +++ b/src/layouts/AppLayout/NavBar/Styling.tsx @@ -1,5 +1,6 @@ import { styled, CSSObject, Theme } from '@mui/material/styles'; -import Drawer from '@mui/material/Drawer'; +import { StyledComponent } from '@emotion/styled'; +import { Box, BoxProps, Drawer, DrawerProps } from '@mui/material'; const PREFIX = 'Navbar'; @@ -13,7 +14,9 @@ export const classes = { // pieChartIcon: `${PREFIX}-pieChartIcon`, }; -export const Root = styled('div', { name: 'NavBar' })(({ theme }) => ({ +// TODO: Explicit type annotation needed until following issue fixed: +// https://github.com/microsoft/TypeScript/issues/48212 +export const Root: StyledComponent = styled(Box, { name: 'NavBar' })(({ theme }) => ({ [`&.${classes.root}`]: { display: 'flex', }, @@ -74,7 +77,10 @@ interface NavDrawerProps { offsetTop: number; } -export const NavDrawer = styled(Drawer, { +// TODO: Explicit type annotation needed until following issue fixed: +// https://github.com/microsoft/TypeScript/issues/48212 +// We also use the second Generic parameter +export const NavDrawer: StyledComponent = styled(Drawer, { shouldForwardProp: (prop) => !['open', 'widthOpen', 'widthClosed', 'offsetTop'].includes(prop as string), })(({ theme, open, widthOpen, widthClosed, offsetTop }) => ({ diff --git a/src/layouts/PageLayout/PageContainer.tsx b/src/layouts/PageLayout/PageContainer.tsx index 4ba3e49..78a759b 100644 --- a/src/layouts/PageLayout/PageContainer.tsx +++ b/src/layouts/PageLayout/PageContainer.tsx @@ -1,7 +1,9 @@ -import { styled, Container } from '@mui/material'; +import { styled } from '@mui/material/styles'; +import { StyledComponent } from '@emotion/styled'; +import { Container, ContainerProps } from '@mui/material'; /** Just a simple styled container applying our spacings*/ -const PageContainer = styled(Container, { +const PageContainer: StyledComponent = styled(Container, { name: 'PageContainer', })(({ theme }) => ({ // Horizontal padding comes from the Container's gutter diff --git a/src/layouts/PageLayout/PanelAwareMargins.tsx b/src/layouts/PageLayout/PanelAwareMargins.tsx index 148a13e..8a40e1c 100644 --- a/src/layouts/PageLayout/PanelAwareMargins.tsx +++ b/src/layouts/PageLayout/PanelAwareMargins.tsx @@ -1,4 +1,6 @@ -import { styled } from '@mui/material'; +import { styled } from '@mui/material/styles'; +import { StyledComponent } from '@emotion/styled'; +import { Box, BoxProps } from '@mui/material'; interface PanelProps { width: number; @@ -15,10 +17,13 @@ interface PanelAwareMarginsProps { rightPanel?: PanelProps; } +// TODO: Explicit type annotation needed until following issue fixed: +// https://github.com/microsoft/TypeScript/issues/48212 + /** With the optional leftPanel and rightPanel props providing state and width, * the children of of the PanelAwareMargins will looks like they is pushed aside * by the panels by transitioning the margins. */ -const PanelAwareMargins = styled('div', { +const PanelAwareMargins: StyledComponent = styled(Box, { shouldForwardProp: (prop) => !(['leftPanel', 'rightPanel'] as Array).includes(prop), name: 'PanelAwareMargins', })(({ theme, leftPanel, rightPanel }) => ({ diff --git a/src/layouts/PageLayout/index.stories.tsx b/src/layouts/PageLayout/index.stories.tsx index d16e3d3..6880722 100644 --- a/src/layouts/PageLayout/index.stories.tsx +++ b/src/layouts/PageLayout/index.stories.tsx @@ -27,7 +27,9 @@ const iconTableItems = [ }, ]; -const meta = { +// TODO: It shouldn't be encessary to have this explicit type annotation here. +// It is a workaround for https://github.com/microsoft/TypeScript/issues/48212 +const meta: Meta = { component: PageLayout, tags: ['autodocs'], parameters: {