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

Typescript support for theme when using css prop #1197

Closed
mgrip opened this issue Jan 27, 2019 · 18 comments
Closed

Typescript support for theme when using css prop #1197

mgrip opened this issue Jan 27, 2019 · 18 comments
Milestone

Comments

@mgrip
Copy link

mgrip commented Jan 27, 2019

  • emotion version: 10.0.6
  • react version: 16.8.0-alpha.1

Relevant code:

<span
  css={theme => ({
    fontWeight: theme.font.weight.light,
    position: "absolute",
    right: "8px",
    top: "52%"
  })}
>

What you did:
Created a theme using using the emotion-theming package

What happened:
No typescript support for the provided theme object.

Reproduction:

Problem description:

From the documentation, it appears the current typescript support allows you to define the theme object shape when using styled(), but not when using the css prop. Currently (and as confirmed by the documentation), the object is typed as any.

Suggested solution:

Not sure if possible, but hopefully provide some way of defining a theme type via typescript generics, even when using the css prop. It appears the type is hardcoded for the css prop to any here

@Ailrun
Copy link
Member

Ailrun commented Feb 1, 2019

Thank you for leaving this issue. Frustratingly, I don't know an appropriate way to apply type parameters to a pre-defined prop.
If you know any ways, please send us a PR! We are welcome your PR :).

(FYI, one possible way is adding some global types for theme and letting users override them, but I think this would make emotion hard to use in libraries, i.e. not appropriate.)

@jaridmargolin
Copy link

(FYI, one possible way is adding some global types for theme and letting users override them, but I think this would make emotion hard to use in libraries, i.e. not appropriate.)

@Ailrun - Do you mind expanding on why this would make emotion hard to use in libraries? I believe this is the approach styled-components took: https://www.styled-components.com/docs/api#create-a-declarations-file

@TarVK
Copy link

TarVK commented Aug 15, 2019

So I have just done a rather extensive work around, since I still haven't found a proper way of doing it. If there is a nice way of removing global declarations in typescript, please let me know.
In any case what I have done is the following:

Dynamically remove emotion's type declaration because typescript complains otherwise.
To do so I have written this script for emotion 10.0.10 (likely to break on updates):

const FS = require("fs");
const Path = require("path");
const emotionFilePath = Path.join("@emotion", "core", "types", "index.d.ts");

// Obtain the possible node_modules file paths by looking at this modul and up the tree.
const paths = [...module.paths];

// Go through all paths
while (paths.length > 0) {
    let path = paths.shift();
    // Check if the file exists
    path = Path.join(path, emotionFilePath);
    if (FS.existsSync(path)) {
        // Get the file
        let fileContents = FS.readFileSync(path, "utf8");

        // Remove the declaration (3 opening brackets followed by 3 closing brackets,
        // with anything inbetween but brackets)
        fileContents = fileContents.replace(
            /declare global \{[^\{\}]*\{[^\{\}]*\{[^\{\}]*\}[^\{\}]*\}[^\{\}]*\}/,
            ""
        );
        // And the same with 2 brackets
        fileContents = fileContents.replace(
            /declare module 'react' \{[^\{\}]*\{[^\{\}]*\}[^\{\}]*\}/,
            ""
        );

        // Write the file
        FS.writeFileSync(path, fileContents, "utf8");
        break;
    }
}

Then called that "dev/removeEmotionJsxGlobal" and assigned it to postinstall in my package:

"scripts": {
    "postinstall": "node dev/removeEmotionJsxGlobal.js"
}

And finally declared my own typings in my index:

declare module "react" {
    interface DOMAttributes<T> {
        css?: InterpolationWithTheme<ITheme>;
    }
}
declare global {
    namespace JSX {
        interface IntrinsicAttributes {
            css?: InterpolationWithTheme<ITheme>;
        }
    }
}

Where ITheme is ofc my theming interface, which can be replaced by your own.

I haven't tested the script on package installation, and think it might fail if modules are installed in the wrong order. I will post an update if I figured that out, but it has been enough frustration for one day in order to get this far.

@jckw
Copy link

jckw commented Aug 22, 2019

Any updates on this?

@dariye
Copy link

dariye commented Sep 21, 2019

Also curious if anyone has found a workaround for this

@mvestergaard
Copy link
Contributor

My suggestion would be that the typings that add the css prop are moved into a separate package that typescript users can install if they don't care about typing the theme.
But if they do, they can define the typings themselves.

@mvestergaard
Copy link
Contributor

Or do something similar to the styled-components typings:

/**
 * The CSS prop is not declared by default in the types as it would cause 'css' to be present
 * on the types of anything that uses styled-components indirectly, even if they do not use the
 * babel plugin.
 *
 * You can load a default declaration by using writing this special import from
 * a typescript file. This module does not exist in reality, which is why the {} is important:
 *
 * ```ts
 * import {} from 'styled-components/cssprop'
 * ```
 *
 * Or you can declare your own module augmentation, which allows you to specify the type of Theme:
 *
 * ```ts
 * import { CSSProp } from 'styled-components'
 *
 * interface MyTheme {}
 *
 * declare module 'react' {
 *   interface Attributes {
 *     css?: CSSProp<MyTheme>
 *   }
 * }
 * ```
 */

@Andarist
Copy link
Member

Andarist commented Nov 4, 2019

@JakeGinnivan could you take a look at this? Seems like this should be solved together with #1249 and optionally with #1507 , right?

@JakeGinnivan
Copy link
Contributor

I think we should do something like this

declare global {
    namespace Emotion {
        export interface Theme { }
    }
}

Then reference Emotion.Theme everywhere instead of any or the Theme generic.

To not break backwards compatibility the user should add (if they were relying on theme being an any atm)

declare global {
    namespace Emotion {
        export interface Theme extends Record<string, any> { }
    }
}

Then in user land you add this somewhere to augment the global module.

declare global {
    namespace Emotion {
        export interface Theme {
            your: 'props'
        }
    }
}

I don't have time this week to prepare a PR for this but if someone wants to have a go, happy to help review if someone wants to give it a go.

@tomsseisums
Copy link
Contributor

May I suggest you introduce this for v11 @Andarist?

@mvestergaard
Copy link
Contributor

@JakeGinnivan I could be wrong, but I think your suggestion may be limiting.

We use material-ui, so would like to expose the Theme type from there.
I don't believe you are able to modify an existing interface to extend another.

@Andarist
Copy link
Member

Andarist commented Nov 5, 2019

@joltmode sure, we plan to add this in v11

@mvestergaard this should work just like anything else, shouldnt it? Do you see any problems with doing this?

declare global {
    namespace Emotion {
        export interface Theme extends MaterialUI.Theme { }
    }
}

@tomsseisums
Copy link
Contributor

tomsseisums commented Nov 5, 2019

@Andarist my hint about v11 was to also add this issue to the milestone. :)

@JakeGinnivan
Copy link
Contributor

@mvestergaard I more was just throwing that out there as a suggestion. It's the one I have used so far with Express and a few other node libraries, but there may be more modern approaches as you suggest.

As mentioned in the above PR, I am unsure if global can be used in babel compilation?

@mvestergaard
Copy link
Contributor

@JakeGinnivan I may be wrong as mentioned. I just have some recollection of possible issues when trying to extend an interface defined in a package. So I just wanted it to be considered when this is worked on, so it isn't limited from the start.

This kinda thing shouldn't have any effect on babel compilation, since babel doesn't do type checking.

@Andarist
Copy link
Member

Andarist commented Nov 6, 2019

This kinda thing shouldn't have any effect on babel compilation, since babel doesn't do type checking.

Such things can have an impact on babel compilation because Babel doesn't really know what a type is and which referenced identifiers are types and which are runtime values. It just uses a number of heuristics to determine that. I think it only matters though in the case of imports - because it needs to remove or leave a particular import so it removes everything that is not referenced as runtime value within a file. As this would be global and always referenced in type positions I think it should just work fine with Babel.

@Andarist
Copy link
Member

Andarist commented Dec 7, 2019

We have decided to include a Theme interface that can be augmented in userland. This also provided that theme to the css prop - it has been implemented in #1609 and will be released in the upcoming v11.

@ReelJohn
Copy link

ReelJohn commented Apr 12, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests