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

Everything resolves to strings #90

Closed
christianalfoni opened this issue Apr 22, 2020 · 12 comments
Closed

Everything resolves to strings #90

christianalfoni opened this issue Apr 22, 2020 · 12 comments

Comments

@christianalfoni
Copy link

christianalfoni commented Apr 22, 2020

Hi there!

It seems to me that all typing resolves to string?

Example:

export type BorderStyleProperty = Globals | LineStyle | string;

Since it can be a string that will override both Globals and LineStyle, not giving intellisense on the possible properties of the border style.

Is this intentional?

I did some more research: microsoft/TypeScript#29729 (comment)

@christianalfoni
Copy link
Author

Btw, this is the proper way to use it:

export type LiteralUnion<T extends U, U = string> = T | (U & {});

@christianalfoni
Copy link
Author

Actually best approach wouod just to use (string | {}) where there is a string now, to not conflict with types having a number in them

@zanona
Copy link

zanona commented Jun 10, 2020

A 'opt-out' type would be great, actually. Such as CSS.PropertiesStrict which wouldn't fallback to string and require assertion of pre-defined values.

I don't use autocompletion, but allowing a strict value set, would certainly help to keep projects under control as tests would fail if using any value type which is not provided by either csstype or extended within the project itself. Commits that use new values types, would have to be declared on the extension file itself, making the reviewing process a breeze.

@remorses
Copy link

Adding the string type to the union does not make much sense imo, you lose auto completion and type validation

@thdxr
Copy link

thdxr commented Jun 23, 2020

Second adding strict properties - a lot is lost with | string

@sjohnsonaz
Copy link

I agree, I'm also losing validation and intellisense because of the | string additions.

@kylemh
Copy link

kylemh commented Oct 7, 2020

@frenic would you be okay with the introduction of a CSS.PropertiesStrict that doesn't allow non-standard values for properties?

@frenic
Copy link
Owner

frenic commented Oct 13, 2020

CSSType v3 new uses the (string | {}) hack which solves the autocomplete problem for properties having union string (| string).

You have to know that not all properties fallbacks with union string. Just the ones that needs it for obvious reasons like multiple values, lengths, URLs, functions etc. So the problem of having a separate "strict" version that doesn't fallback to union string at all is that it will result in lots of type errors for simple style like any of these:

{
  color: '#ffffff',
  backgroundImage: 'url(path/to/an/image.jpg)',
  fontSize: '10px',
}

There's lots of examples like this were it will be more of a problem not having union string.

@frenic
Copy link
Owner

frenic commented Oct 13, 2020

I'm closing this because the original post was related to the autocompletion issue that is now fixed

@frenic frenic closed this as completed Oct 13, 2020
@kylemh
Copy link

kylemh commented Oct 13, 2020

Should I open a new issue? My qualm is with properties like display allowing any string.

@frenic
Copy link
Owner

frenic commented Oct 16, 2020

@kylemh display can have combinations of values https://drafts.csswg.org/css-display/#the-display-properties so it would be wrong to remove | string from that property. I've given it some attempts to combine different values into multiple string literals and in that way be able to skip | string but it's really complicated to do problematically and the type definitions exploded in size and performance issues became a concern so the progress halted.

I may give it another try soon thanks to this which will make things a bit easier. At least support a fragment of different combinations so display becomes complete without | string would be really nice. I'm well aware of the issue but you're welcome to submit a separate issue if you'd like.

@kylemh
Copy link

kylemh commented Oct 16, 2020

Understood. Thanks so much for the full context. Is there an issue or PR I can follow regarding the potential new attempt with contexts + template literals.

@heleg heleg mentioned this issue Jul 30, 2023
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

7 participants