Skip to content

Commit

Permalink
[EVNT-59] Workaround for typescript issue microsoft/TypeScript#48212
Browse files Browse the repository at this point in the history
  • Loading branch information
anthonyblond committed Aug 21, 2023
1 parent 12450d9 commit f4fffa3
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 12 deletions.
49 changes: 49 additions & 0 deletions DEVELOPMENT.md
Expand Up @@ -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:
<https://github.com/microsoft/TypeScript/issues/48212>, 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<BoxProps> = styled(Box, {
name: 'NavBar',
})(({ theme }) => ({
// ... object containing styling
}));
```

## Yarn/npm linking

TODO: Currently experimenting with [pnpm](https://pnpm.io) so this is out of
Expand Down
13 changes: 10 additions & 3 deletions 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;
Expand All @@ -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<BoxProps> = styled(Box, {
name: 'NavBar',
})(({ theme }) => ({
[`&.${classes.root}`]: {
display: 'flex',
},
Expand Down Expand Up @@ -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<DrawerProps> = styled(Drawer, {
shouldForwardProp: (prop) => prop !== 'open',
})(({ theme, open }) => ({
width: open ? NAVBAR_WIDTH_OPENED : NAVBAR_WIDTH_CLOSED,
Expand Down
4 changes: 3 additions & 1 deletion src/components/NavBar/index.stories.tsx
Expand Up @@ -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<typeof NavBar> = {
component: NavBar,
tags: ['autodocs'],
decorators: [
Expand Down
12 changes: 9 additions & 3 deletions 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';

Expand All @@ -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<BoxProps> = styled(Box, { name: 'NavBar' })(({ theme }) => ({
[`&.${classes.root}`]: {
display: 'flex',
},
Expand Down Expand Up @@ -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<DrawerProps, NavDrawerProps> = styled(Drawer, {
shouldForwardProp: (prop) =>
!['open', 'widthOpen', 'widthClosed', 'offsetTop'].includes(prop as string),
})<NavDrawerProps>(({ theme, open, widthOpen, widthClosed, offsetTop }) => ({
Expand Down
6 changes: 4 additions & 2 deletions 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<ContainerProps> = styled(Container, {
name: 'PageContainer',
})(({ theme }) => ({
// Horizontal padding comes from the Container's gutter
Expand Down
9 changes: 7 additions & 2 deletions 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;
Expand All @@ -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<BoxProps> = styled(Box, {
shouldForwardProp: (prop) => !(['leftPanel', 'rightPanel'] as Array<PropertyKey>).includes(prop),
name: 'PanelAwareMargins',
})<PanelAwareMarginsProps>(({ theme, leftPanel, rightPanel }) => ({
Expand Down
4 changes: 3 additions & 1 deletion src/layouts/PageLayout/index.stories.tsx
Expand Up @@ -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<typeof PageLayout> = {
component: PageLayout,
tags: ['autodocs'],
parameters: {
Expand Down

0 comments on commit f4fffa3

Please sign in to comment.