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

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

Open
jisaacks opened this issue Sep 14, 2022 · 6 comments
Open

Comments

@jisaacks
Copy link

jisaacks commented Sep 14, 2022

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
Copy link
Owner

frenic commented Sep 15, 2022

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
Copy link
Author

@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
Copy link

Any fresh thoughts on this?

@bertin0
Copy link

bertin0 commented Nov 22, 2022

@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
Copy link

ndp commented Mar 2, 2023

Duplicated here #166

@muchisx
Copy link

muchisx commented Jun 21, 2023

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.

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

No branches or pull requests

6 participants