Definitelytyped: [jss] [bug] typing issues when writing JSS.

Created on 12 Apr 2018  ·  13Comments  ·  Source: DefinitelyTyped/DefinitelyTyped

If you know how to fix the issue, make a pull request instead.

  • [x] I tried using the @types/xxxx package and had problems.
  • [x] I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
  • [x] I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
  • [x] [Mention](https://github.com/blog/821-mention-somebody-they-re-notified) the authors (see Definitions by: in index.d.ts) so they can respond.

    • Authors: @frenic @pelotom @daugsbi @DanielRosenwasser @appsforartists @andy-ms @SergioMorchon

Hi,
I come from the Material UI (Mui) community, and they made use of jss. However, I encountered a problem writing JSS in a Mui based project. I believe this issue directly relates to JSS typings. Please see below.

Expected Behavior

No type errors should be reported for adding legal styles kv pairs in withStyles()

Current Behavior


For some style properties, like position, overflow, etc, specifying a valid value, e.g., position: 'absolute' or overflow: 'auto' reports type errors.
Example:
image

The only hack is to use as:
image

Most helpful comment

There’s nothing wrong with the typings here, TypeScript’s type widening is the issue. I don’t know what to say except type widening sucks and I wish it didn’t exist or could be turned off 🤷‍♂️

All 13 comments

There’s nothing wrong with the typings here, TypeScript’s type widening is the issue. I don’t know what to say except type widening sucks and I wish it didn’t exist or could be turned off 🤷‍♂️

That was an awfully quick and disappointing close.
Are you really sure this is where you want to leave this?
I've no special insight, but I doubt help is on the way from Typescript.

@estaub How would you suggest JSS should change to avoid this problem? Go back to having weak typings for CSS? I'm really not sure what you're proposing.

@estaub FWIW, the OP is the one who closed the issue. Sounds like he's satisfied with the response.

If the only two possibilities were the current implementation, requiring "as" as shown above, and weak typings, then yes, I'd go with weak typings; IMHO this creates an onerous burden that deprecates Typescript as a whole. But fortunately, there are other options, most of which I'm sure I haven't thought of. But here are three, more properly in a csstype issue, but here we are. These aren't mutually exclusive

The first option is to publish variants of the typings that vary in strictness. People can choose their desired level.

The second is to provide a string-enum-based alternative syntax:

In csstype:

enum GLOBALS {
    inherit="inherit",
    initial="initial",
    unset="unset"
}
enum POSITION {
    absolute="absolute",
    fixed="fixed",
    relative="relative",
    static="static",
    sticky="sticky"
}

type PositionProperty = GLOBALS | POSITION | 
    Globals | "absolute" | "fixed" | "relative" | "static" | "sticky";

Then in user code:

import {POSITION, ...}
const sheet = { a: { b: POSITION.absolute }}

The third alternative is to offer something like the workaround I'm using now. To be honest, I'm not sure _how_ it prevents type-widening at the point of checking; maybe someone can explain. It has a bug in that it fails to capture capture the keys; e.g. the example at the end has a type of StyleShete<string>, not StyleShete<'x'> as I'd like.

// The stuff within the braces in CSS (including the braces)
export type CSSDeclBlock = CSS.Properties<string | number>

// Misspelled to avoid collision with libs.
// The generic T is used to allow inferring of the set of names;
// see checkStyle, below
export type StyleShete<T extends string> = {
    [key in T]: CSSDeclBlock;
}

// creates a function that compile-time ensures that its arg is of a given type,
//  then passes it through 
// ref: https://stackoverflow.com/questions/43902192/can-i-both-declare-a-broad-type-and-use-inferred-type-in-typescript
export function fCheckType<DeclaredType>() {
    return function <InferredType extends DeclaredType>(t: InferredType): InferredType {
        return t
    }
}
export function checkStyle<T extends string>(ss: StyleShete<T>) {
    return fCheckType<StyleShete<T>>()(ss)
}

const checkedStyle = checkStyle({x: {textAlign:'center'}})
const iGotNoStyle = checkStyle({x: {textAlign:'middle'}})  // Type 'middle' is not assignable...

It may be possible to improve the second or third option using the new conditional type support in Typescript 2.8; I keep looking hungrily at the infer keyword but haven't found a recipe. Trivially, the third option could use the new ReturnType generic.

The first option is to publish variants of the typings that vary in strictness. People can choose their desired level.

You can easily do this yourself with module augmentation.

The second is to provide a string-enum-based alternative syntax:

Enums are not an option for CSSType because they require a runtime component.

@pelotom

You can easily do this yourself with module augmentation.

C'mon. If you want to go mainstream, you have to aim for an audience that expects it to work well out of the box. You don't just have early adopters to think about now. Most csstype users will have never heard of it, let alone read the readme. Mind you, I'm not married to publishing variants - it may well be a bad idea! BTW, it might also be possible to control strictness with another generic parameter to Properties.

Enums are not an option for CSSType because they require a runtime component.

Sorry, I'm not understanding why that excludes them as an option.
Is it because being a devDependency is a requirement for csstype?

C'mon. If you want to go mainstream, you have to aim for an audience that expects it to work well out of the box. You don't just have early adopters to think about now. Most csstype users will have never heard of it, let alone read the readme. Mind you, I'm not married to publishing variants - it may well be a bad idea! BTW, it might also be possible to control strictness with another generic parameter to Properties.

Is it much easier for downstream users to know that they need to use @types/weaky-typed-jss instead of @types/jss? Beyond that, if what you're actually using directly is material-ui, how can you direct material-ui to use @types/weakly-typed-jss instead of @types/jss? This is the use case module augmentation was invented for, because it globally changes the definitions for all parties depending on them. I'm not being facetious, this is the simplest solution I know of.

Sorry, I'm not understanding why that excludes them as an option.
Is it because being a devDependency is a requirement for csstype?

CSSType is nothing but static type definitions, it carries no runtime payload. Likewise, it is depended on by purely static definitions like @types/react and @types/jss. To make CSSType a runtime library means it can't be used by those pure type definitions without making the corresponding runtime libraries (react and jss) also depend on csstype.

In more concrete terms:

enum Position {
  ABSOLUTE = 'absolute',
}

createStyleSheet({
  position: Position.ABSOLUTE,
});

gets converted to JS

const Position = {
  ABSOLUTE: 'absolute',
};

createStyleSheet({
  position: Position.ABSOLUTE,
});

However, the enum would be published in the typings:

// imaginary jss.d.ts
export enum Position {
  ABSOLUTE = 'absolute',
}

// your code
import {Position} from 'jss';
createStyleSheet({
  position: Position.ABSOLUTE,
});

You're importing jss, and jss doesn't export an enum called Position. When you tried to run that code, you'd get a "Cannot read property 'ABSOLUTE' of undefined" error.

Would there be some way to make module augmentation trivial for different levels of strictness? I'm not sure what I mean; I'm not good with Typescript decls. But I'm thinking that one might override just a declaration or two to get varying levels of strictness across a common range.

_If the enum idea is otherwise useful_, it overlaps with the variant idea; it _might_ make sense to publish one no-runtime for inclusion by other @types, and one with them for inclusion elsewhere. I'm just spitballing this, of course!

Just started migrating a large codebase to TypeScript, and with approximately zero type annotations added so far, we're hitting this. We use JSS indirectly via @material-ui.

I don't think the types should be loosened so that Position just means string. It's not like the other string CSS attributes are checkable, and trying to enforce a string whitelist in this case is causing problems that aren't worth it.

In the mean time, we're probably going to wrap withStyles in a function that casts to any before passing the styles along.

Actually, it looks like material-ui has a solution to this; apparently the issue only arises if there's an assignment-to-inferred-type-variable in between the styles and the style getting checked, so they wrap it in a no-op function. https://material-ui.com/guides/typescript/

I came across this issue while trying to solve this problem using JSS for the first time. One way is to use const assertions (introduced in TS 3.4), e.g.

const foo = {
  postition: "absolute"
} as const

will narrow the type of each prop to the literal strings, rather than string which satisfies JSS types. Really simple.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tyv picture tyv  ·  3Comments

JWT
svipas picture svipas  ·  3Comments

victor-guoyu picture victor-guoyu  ·  3Comments

demisx picture demisx  ·  3Comments

variousauthors picture variousauthors  ·  3Comments