frenic / csstype

Strict TypeScript and Flow types for style based on MDN data
MIT License
1.73k stars 72 forks source link

values that should be <length> | <percentage> accept any string as a value #163

Open jisaacks opened 2 years ago

jisaacks commented 2 years ago

Example:

import { Properties } from 'csstype'

const style: Properties = {
  color: 'white',
  paddingRight: 'anything allowed here', // Should be a type error but not
}

When I look up the definition it is like this:

export type PaddingRight<TLength = (string & {}) | 0> = Globals | TLength | (string & {});

Which essentially allows any string. I think a better approach would be something like this:

type units = 'em' | 'rem' | 'px' // | ... (full list found here: https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units

type LengthOrPercentage = `${number}${unit}` | 0

export type PaddingRight<TLength extends LengthOrPercentage> = Globals | TLength;

This would need to be updated for a lot of properties tho, not just paddingRight.

Is this a change you would be interested in?

frenic commented 2 years ago

This would be great! String templates is something that should be introduced. But concerning properties that contains <length> is that calc() is allowed. So even if it uses string templates for <length> it still needs to have "any" string.

jisaacks commented 2 years ago

@frenic I see what you mean, I did not consider calc. However you could allow any string only inside the calc() function.

here is an example: playground

This allows for single values such as paddingRight or up to 4 values such as padding if you use calc() for a value, it will allow any string to be calculated but otherwise enforces correct number/units.

However, the types are becoming too deep. Not sure if that can be fixed. So I can understand this being undesirable.

MrFoxPro commented 1 year ago

Any fresh thoughts on this?

bertin0 commented 1 year ago

@frenic I see what you mean, I did not consider calc. However you could allow any string only inside the calc() function.

here is an example: playground

This allows for single values such as paddingRight or up to 4 values such as padding if you use calc() for a value, it will allow any string to be calculated but otherwise enforces correct number/units.

However, the types are becoming too deep. Not sure if that can be fixed. So I can understand this being undesirable.

This seems like a nice solution.

ndp commented 1 year ago

Duplicated here https://github.com/frenic/csstype/issues/166

muchisx commented 1 year ago

Any updates ont this? The repo ReadMe describes the lib as strict typing but seems like properties can just be any string, which defeats the purpose of even installing this lib.