Skip to content

Strict mode and review CSS Types #206

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 17, 2020
Merged

Strict mode and review CSS Types #206

merged 1 commit into from
Sep 17, 2020

Conversation

peduarte
Copy link
Contributor

@peduarte peduarte commented Sep 10, 2020 •
edited
Loading

Taking over from #116

This PR adds a strict mode feature, originally proposed by @fgnass

Strict mode is false by default.

I also did a review of the CSS Types, which closes #48. For example, this allows numbers in margin properties, since internally we convert them to px.

strictmode

Important note

When strict: true shorthands have a stricter syntax:

image

AFAIK its not possible to type strings that can contain multiple values.

Unless, somehow, we create all the possible combinations of all tokens? (sounds like a terrible, terrible idea)

Say you have 4 space tokens: $1, $2, $3, $4

And you wanna add margin on all sides, you can do:
margin: "$2" // this passes validation, because its a single value in a string and its a token

Now you wanna set margin top/bottom and margin left/right
margin: "$2 $4" // As far as TS is concerned, this is still a single string so it cant offer you autocomplete

The only way would be to construct all the possible strings:

"$1"
"$2"
"$3"
"$4"
"$1 $1"
"$1 $2"
"$1 $3"
"$1 $4"
"$1 $1 $1"
"$1 $1 $2"
"$1 $1 $3"
"$1 $1 $4"
"$1 $1 $1 $1"
"$1 $1 $1 $2"
"$1 $1 $1 $3"
"$1 $1 $1 $4"
"$2 $1"
"$2 $2"
"$2 $3"
"$2 $4"
etc....

@peduarte peduarte mentioned this pull request Sep 10, 2020
Closed
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d8033f6:

Sandbox Source
Stitches CI: Next.js Configuration
Stitches CI: CRA Configuration

@drewbourne
Copy link

If I understand it correctly this new TypeScript feature may allow for typing the shorthands: microsoft/TypeScript#40336

@peduarte
Copy link
Contributor Author

@drewbourne i thought so too, but after speaking briefly with @benoitgrelard it doesn't seem like it.

But we could be wrong.

If that feature works for us, that would be amazing!

@benoitgrelard
Copy link

At first glance I didn't think so, but yeah I could be totally wrong.
I'm not sure I understand it enough to be completely honest!

On a second look, it does seem that TS would be able to infer parts of a string (the Split example they give is quite telling for example: type T41 = Split<'foo.bar.baz', '.'>; // ['foo', 'bar', 'baz'])

I need to take a deeper read…
Do we know if this is released or when it is due to be released?

@peduarte
Copy link
Contributor Author

@benoitgrelard I think it's now in v4.1.0-dev.20200911 https://www.npmjs.com/package/typescript/v/4.1.0-dev.20200911

@dirtyrolf
Copy link

dirtyrolf commented Sep 12, 2020 •
edited
Loading

Yes, the current typescript@next version supports this!
image

@peduarte
Copy link
Contributor Author

@dirtyrolf daaamn that is slick! Thanks for looking into it!

@peduarte peduarte mentioned this pull request Sep 16, 2020
Closed
@peduarte peduarte merged commit a6dda64 into canary Sep 17, 2020
@peduarte peduarte deleted the strict-mode branch September 17, 2020 10:13
@peduarte peduarte mentioned this pull request Sep 17, 2020
Open
@hasparus hasparus mentioned this pull request Sep 30, 2020
Closed
5 tasks
@ivanbanov ivanbanov mentioned this pull request Oct 25, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christianalfoni christianalfoni Awaiting requested review from christianalfoni

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Review typing of CSS values
4 participants
@peduarte @drewbourne @benoitgrelard @dirtyrolf