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.

Sign up for GitHub

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

fix(types): restrict regular styles function call to one argument #38

Merged
merged 1 commit into from
Oct 10, 2022
Merged

fix(types): restrict regular styles function call to one argument #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
95 changes: 52 additions & 43 deletions src/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,41 @@
import { Properties as CSSProperties } from "csstype";
import { JSX } from "solid-js";
import { JSX, Component } from "solid-js";

// This type is only available in Solid 1.5 or later.
type ValidComponent =
| keyof JSX.IntrinsicElements
| Component<any>
| (string & {});

export interface DefaultTheme {}
export interface CSSAttribute extends CSSProperties {
[key: string]: CSSAttribute | string | number | undefined;
}
type StylesGenerator<P = {}> =
(props: P) => CSSAttribute | string;
type TagStyleGenerator<P = {}> =
(props: P) => number | string | undefined;
type TagArgs<P = {}> = Array<string | TagStyleGenerator<P>>;
type StylesArg<P = {}> =
| string
| CSSAttribute
| StylesGenerator<P>
| Array<CSSAttribute | StylesGenerator<P>>;
export declare function keyframes(styles: StylesArg): string;
export declare function keyframes(
tag: TemplateStringsArray | string,
...props: Array<string | number>
tag: TemplateStringsArray,
...tagArgs: TagArgs
): string;
export declare function extractCss(): string;
export declare function glob(styles: StylesArg): void;
export declare function glob(
tag: CSSAttribute | TemplateStringsArray | string,
...props: Array<string | number>
tag: TemplateStringsArray,
...tagArgs: TagArgs
): void;
export declare function css(styles: StylesArg): string;
export declare function css(
tag: CSSAttribute | TemplateStringsArray | string,
...props: Array<string | number>
tag: TemplateStringsArray,
...tagArgs: TagArgs
): string;
Comment on lines +35 to 39
Copy link
Contributor Author

@100terres 100terres Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

By having multiple declarations we can restrict the use of multiple arguments to the "tag function" and make sure that regular function call are restircted to one argument.

// allowed
css`
  color: ${theme.typography.color};
`
// allowed
css({
  color: ${theme.typography.color},
});

// With the current implementation the following is allow
// but with this PR the following would be disallowed
css(
  {
    color: theme.typography.color,
  },
  // When we are not using a tag function,
  // only the first argument is used by goober
  // The following won't be added to goober
  {
    font-family: theme.typography.fontFamily,
  },
);

export declare function shouldForwardProp(
predicate: (x: string) => boolean
Expand All @@ -31,46 +51,35 @@ export declare function ThemeProvider<
}
>(props: T): JSX.Element;
export declare function useTheme(): DefaultTheme;
type Tagged<T> = <P>(
args_0:
| string
| TemplateStringsArray
| CSSAttribute
| ((
props: P &
T & {
theme?: DefaultTheme;
as?: string | number | symbol | undefined;
class?: any;
children?: any;
}
) => string | CSSAttribute),
...args_1: (
| string
| number
| ((
props: P &
T & {
theme?: DefaultTheme;
as?: string | number | symbol | undefined;
class?: any;
children?: any;
}
) => string | number | CSSAttribute | undefined)
)[]
) => ((props: P & T) => JSX.Element) & {
class: (props: P & T) => string;
export interface ThemeProp {
theme?: DefaultTheme;
}
Comment on lines +54 to +56
Copy link
Contributor Author

@100terres 100terres Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is would allow us to override the props in a project with a theme with something like this:

// in a styled.d.ts file
declare module "solid-styled-components" {
  export interface DefaultTheme {
    // ...
  }

  // The theme will always be present
  export interface ThemeProp {
    theme: DefaultTheme;
    //   ^
    // The `?` was removed 
  }

We won't need to make sure that the theme exist on the props e.g.

const Button = styled("button")((props) => ({
  color: props.theme.typography.color,
  //          ^
  // we won't need to check if the theme is present or not
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could extract this in an other PR if needed.

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 is not possible with the current version of TypeScript. I've added a comment on an existing issue on the TypeScript repo: microsoft/TypeScript#33699 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now using the non-null assertion operator should do the trick:

styled("span")`
  color: ${(props) => props.theme!.palette}
`;

interface AsProps {
as?: ValidComponent;
}
type StylesFn<T> = <P>(
styles: StylesArg<P & T & AsProps & ThemeProp>,
) => Component<P & T & AsProps> & {
class: (props: P & T & AsProps) => string;
};
type TagFn<T> = <P>(
tag: TemplateStringsArray,
...args: TagArgs<P & T & AsProps & ThemeProp>,
) => Component<P & T & AsProps> & {
class: (props: P & T & AsProps) => string;
};
type StylingFn<T> = StylesFn<T> & TagFn<T>;
export interface Styled {
<T extends keyof JSX.IntrinsicElements>(
tag: T | ((props: JSX.IntrinsicElements[T]) => JSX.Element)
): Tagged<JSX.IntrinsicElements[T]>;
<T>(component: (props: T) => JSX.Element): Tagged<T>;
element: T | Component<JSX.IntrinsicElements[T]>
): StylingFn<JSX.IntrinsicElements[T]>;
<T>(component: Component<T>): StylingFn<T>;
}
export declare const styled: Styled & {
[Tag in keyof JSX.IntrinsicElements]: Tagged<JSX.IntrinsicElements[Tag]>;
[element in keyof JSX.IntrinsicElements]: StylingFn<JSX.IntrinsicElements[element]>;
};
export declare function createGlobalStyles(
tag: CSSAttribute | TemplateStringsArray | string,
...props: Array<string | number | Function>
): Function;
tag: TemplateStringsArray,
...tagArgs: TagArgs
): () => null;
export declare function createGlobalStyles(styles: StylesArg): () => null;
Comment on lines +83 to +85
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the theme is available in the createGlobalStyles. I'll have a look and see if it can be added.

26 changes: 26 additions & 0 deletions test/test.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,32 @@ describe("Simple Styled", () => {
});
});

test("Creates component properly with styles function", () => {
const Div = styled("div")<{ bold: boolean; border: number; color: string }>(({ bold, border, color }) => ({
color: "steelblue",
fontSize: "32px",
padding: "5px",
border: `${border}px solid ${color}`,
backgroundColor: "linen",
fontWeight: (bold ? "bold" : 100),
}));

createRoot(() => {
const v = (
<Div
aria-label="button"
onClick={() => {}}
class="test"
bold={true}
border={1}
color="whitesmoke"
>
Testera
</Div>
);
});
});

test("Creates input properly", () => {
const Input = styled("input")`
width: 5em;
Expand Down