-
Notifications
You must be signed in to change notification settings - Fork 209
Add Notes to readme around limitations of typescript support #83
Comments
Thanks for opening the issue and suggestion! PR certainly welcome! // @luke-john |
👍 I'll definetely take a look at it if you open the PR |
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;
0 support
only have support for div and svg
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
possible support via glamors typings https://github.com/threepointone/glamor/blob/master/index.d.ts |
I think we should hold off writing definitions for built-in Glamorous components because of this. |
@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 :). |
@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. |
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. |
@sammkj Aaah sorry, yes I have mistaken it. I see. |
glamorous
version: master@20170424Problem 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 👍 .
The text was updated successfully, but these errors were encountered: