Use a better approach on types/styles

[pedrobslisboa]

Problem

import * as S from './styled'
import * as I from './types'

@marcelines and I talked about it, and we think that it could be a better way to deal with, improving legibility.

Imagine if you need to import another type on this file:

import * as S from './styled'
import * as I from './types'
import * as T from '../Card/types' //?????? 

The code below is from a 'type' file, and it has import * as I ...

import * as I from '../../types/orders'

export type OrderTitle = {
  id: string,
  insertedAt: string,
  status: I.OrderStatus
}

export type OrderSummary = {
  total: number,
  totalBeforeVat: number,
  totalVat: number,
  packagingPostageTotal: number
}

Same problem. Imagine if you need to import another type on this file:

import * as I from '../../types/orders'
import * as T from '../../types/categories' // ??????

export type OrderTitle = {
  id: string,
  insertedAt: string,
  status: I.OrderStatus
}

export type OrderSummary = {
  total: number,
  totalBeforeVat: number,
  totalVat: number,
  packagingPostageTotal: number
}

Solution

We can do like this:

import * as Styled from './styled'
import * as CardStyled from '../Card/styled'

import * as Types from './types'
import * as CardTypes from '../Card/types'

import * as OrderTypes from '../../types/orders'
import * as CategoriesTypes from '../../types/categories'

export type OrderTitle = {
  id: string,
  insertedAt: string,
  status: OrderTypes.OrderStatus
}

export type OrderSummary = {
  total: number,
  totalBeforeVat: number,
  totalVat: number,
  packagingPostageTotal: number
}

or:

import { StyledGood } from './styled'
import { StyledCard, StyledButton } from '../Card/styled'

import { GoodType, PropsType } from './types'
import { CardType } from '../Card/types'

import { OrderType } from '../../types/orders'
import { CategoriesType } from '../../types/categories'

export type OrderTitle = {
  id: string,
  insertedAt: string,
  status: CategoriesType
}

export type OrderSummary = {
  total: number,
  totalBeforeVat: number,
  totalVat: number,
  packagingPostageTotal: number
}

[pedrobslisboa]

• Named import
  We can have some conflicts with names

• Import all
  We'll import all, even if we want only one styled/type.

I believe, maybe, named import is a better approach since we follow some rules for it. Such as suffix or prefix to identify then as Styles and Types.

[pdrbrnd]

Just my two cents:

I loved it when we started doing import * as S from './styled' but mainly because it felt a bit like plain old CSS (i.e.: styles in their own files), with the advantage of being JS (theme accessible everywhere, functions to mutate style, etc.).
That being said, I openly admit that it requires too much discipline on the developer side to:

• make sure they're using all the named exports on the styled file (as they're all imported by the wildcard)
• try to always have one to one file. i.e.: avoid importing styles from other unrelated files. (Foo.tsx and Foo.style.ts only)

so, basically, this approach requires maybe too much from the developer to keep things organised and maintainable.
Example:

• Dev1 creates SomeCard.tsx and SomeCard.style.ts
• Dev2 creates another component and (wrongly?) imports styles from SomeCard.style.ts
• Dev1 refactors and changes some styled-components in SomeCard.
• problems

---

I think it's sensible to keep everything named export (as it currently is) but avoid wildcard imports.
I think it's also sensible to improve the guidelines to define some rules:

• Prefixing styled components with Styled*?
• When to create new style files and when to centralize styles for multiple components? And how to make it evident that that's the case (i.e.: make it clear that this style file is used by more than one component)

[danilowoz]

I totally agree with the @pbrandone concerns about the maintainability of the components. At the beginner, the wildcard import looked a great idea, but definitely it doesn't fit for big projects where more than one developer will work.

Also, I used to be against the idea of the prefix (it seemed redundant), but from now it makes sense for me @pedrobslisboa, thanks to bringing this thread. Using this pattern, I'm able to know if can use styled props in the component, like as one.

---

Since we all agree that named export is a good idea for the components, I would go forward and only use this pattern to export components, in another word, we must avoid default exports too in order to not end up with the same issue: import from the same file with different names.

So, my suggestion of the ""perfect"" case would be:

import styled from "styled-components"

export const StyledCard = styled.div``

export const ListCard = () => (
  <>
    <StyledCard />
    <StyledCard />
    <StyledCard />
  </>
)

import { StyledCard, ListCard } from "./"

What do you think guys?

[pdrbrnd]

@danilowoz @pedrobslisboa

my question now would be:

• should we create a separate file for styles or keep them together with the JSX (in the bottom of the file)?

[pedrobslisboa]

I agree with @danilowoz, having styled-components on our JSX file we'll be able to perform knowing which is the styled of the component and which is an outsider styled. The same, I think about types.

But I don't agree with the default export, cause maybe I didn't catch the problem with it. Because it doesn't bother me to read:

import styled from "styled-components"

export type PropsType = { // It shouldn't be ListCardProps because is redundant
  orientation: 'hotizontal' | 'vertical'
}

export const StyledCard = styled.div``

const ListCard = (props: Props) =>
  const { orientation = 'vertical' } = props;  
  return (
    <>
      <StyledCard oriantation={orientation} />
      <StyledCard oriantation={orientation} />
      <StyledCard oriantation={orientation} />
    </>
  )

export default ListCard

import ListCard, { StyledCard, PropsType as ListCardProps } from "../ListCard" // I know ListCard is the main component
// or
import ListCard from "../ListCard"
// and
import type { PropsType as ListCardPropsType } from "../ListCard" // for flow

[pdrbrnd]

@pedrobslisboa I think you meant:

const ListCard = ...

export default ListCard

(also it's flow that requires import type, not typescript.)

but I agree with the gist of it and I don't see a problem with exporting as default the main component.

If we are to write styled-components in the same file, I would suggest keeping them at the end.

[pedrobslisboa]

@pedrobslisboa I think you meant:
(also it's flow that requires import type, not typescript.)

Both fixed 👍

Agree to keep all at the end

[danilowoz]

In order to close this issue, @pedrobslisboa could you please open a new PR adding some sample code into the boilerplate? Then the developer who uses the @significa/start will be able to follow this new "rule".