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

fix: jssStyle type #1423

Merged
merged 2 commits into from
Nov 18, 2020
Merged

fix: jssStyle type #1423

merged 2 commits into from
Nov 18, 2020

Conversation

hosseinmd
Copy link
Member

typescript can't work with [K in keyof NormalCssProperties | string]

Corresponding issue (if exists):

What would you like to add/fix?

type definition

Todo

  • Add tests if possible
  • Add changelog if users should know about the change
  • Add documentation

@kof
Copy link
Member

kof commented Nov 13, 2020

What is the actual error compiler was reporting? This fix seems fishy, need some ts experts to confirm cc @moshest

@hosseinmd
Copy link
Member Author

the bug is where intellisense no suggestions can't suggest CSS Properties.

@hosseinmd
Copy link
Member Author

Because we tell them k is CSS properties or string, which means any string.

@kof
Copy link
Member

kof commented Nov 13, 2020

oh I see, that makes sense

@moshest
Copy link
Member

moshest commented Nov 15, 2020

Why we remove there the Observable<NormalCssValues<K> | JssStyle | undefined> part?

typescript can't work with   [K in keyof NormalCssProperties | string]
@hosseinmd
Copy link
Member Author

Why we remove there the Observable<NormalCssValues<K> | JssStyle | undefined> part?

Hi, You are right, that is my mistake, I fix that.

packages/jss/src/index.d.ts Outdated Show resolved Hide resolved
@hosseinmd
Copy link
Member Author

@kof please review the last commit

@moshest
Copy link
Member

moshest commented Nov 18, 2020

lgtm

@kof kof merged commit 3c33514 into cssinjs:master Nov 18, 2020
@kof
Copy link
Member

kof commented Nov 18, 2020

merged, thanks

@hosseinmd
Copy link
Member Author

Could you please publish master?

@hosseinmd hosseinmd deleted the patch-1 branch January 13, 2021 12:58
@hosseinmd
Copy link
Member Author

Please publish

@kof
Copy link
Member

kof commented Jan 23, 2021

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

Successfully merging this pull request may close these issues.

3 participants