Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Add Notes to readme around limitations of typescript support #83

Closed
luke-john opened this issue Apr 23, 2017 · 8 comments · Fixed by #122
Closed

Add Notes to readme around limitations of typescript support #83

luke-john opened this issue Apr 23, 2017 · 8 comments · Fixed by #122

Comments

@luke-john
Copy link
Collaborator

Problem description:

The current bundled typescript definitions are incomplete and based around the needs of the developers who contributed them.

Whilst this is true of the definitions of many non typescript projects, it could still be a pain point for consumers.

Suggested solution:

It would nice to add a small section to the README with a brief overview of what the definitions do and don't include currently.

I'll likely be able to open a PR in a few days, however if someone is able to work on this before then, that would be great 👍 .

@paulmolluzzo
Copy link
Collaborator

Thanks for opening the issue and suggestion! PR certainly welcome!

// @luke-john

@ErikCupal
Copy link
Collaborator

👍 I'll definetely take a look at it if you open the PR

@luke-john
Copy link
Collaborator Author

Sorry I'd forgotten about this one. Though in the week since creation a some of the limitations have been solved :) .

Currently the main ones are as follows;

built-in GlamorousComponents

0 support

built-in DOM component factories

only have support for div and svg

  • pseudo-class
  • pseudo-element
  • Relational CSS Selectors
  • Media Queries

All of these work, however you only get typesafety and intellisense on simple css key props (see https://github.com/paypal/glamorous/blob/master/typings/css-properties.d.ts).

potentially we could add support via helper methods along the lines of http://typestyle.io/. probs needs discussion.

Also as @ErikCupal pointed out in pr this may become possible in future with regex typings microsoft/TypeScript#6579

  • Animations

possible support via glamors typings https://github.com/threepointone/glamor/blob/master/index.d.ts

@kokjinsam
Copy link
Collaborator

I think we should hold off writing definitions for built-in Glamorous components because of this.

@ErikCupal
Copy link
Collaborator

@sammkj Well, I don't think there is a problem with library size. The typescript typings are never included in the final module (either as single js file or as part of webpack build). They simply cannot add any library size because they are just linting informations. The tests, examples or docs also don't add up the size. Hope it's clear :).

@kokjinsam
Copy link
Collaborator

@ErikCupal ah, I think you have mistaken my reference to the issue. If you look through the issue, there might be a change in glamorous APIs. I was just thinking we should wait and see before we write more definitions.

@kentcdodds
Copy link
Contributor

kentcdodds commented May 18, 2017

Ah, I should have mentioned, all the changes to the glamorous APIs have been made. The full API is now:

glamorous.div(
  {}, // objects
  undefined,
  null,
  'class-name', // strings (this was the only change from what was already there
  (props, theme) => ({}), // functions that return objects
  (props, theme) => ('class-name2') // functions that return strings, I guess that is a change too...
)

I don't expect the API to change from that ever.

@ErikCupal
Copy link
Collaborator

@sammkj Aaah sorry, yes I have mistaken it. I see.
@kentcdodds Ok, thanks :)
@sammkj @luke-john It looks we can start writing typings since all API changes are made. Am I right?

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

Successfully merging a pull request may close this issue.

5 participants