Skip to content

converting view to typescript #714

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

Merged
merged 5 commits into from
Mar 23, 2020
Merged

Conversation

Niryo
Copy link
Contributor

@Niryo Niryo commented Mar 21, 2020

again, skip the autogenerated typings, but don't skip the typings that are in the "typings" dir (the manual ones).

Copy link
Collaborator

@ethanshar ethanshar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Niryo
I went over everything, I asked some questions there..
anyway, there's a bug, the View example screen and TextField example screen are throwing error.
maybe there are more screens. I will try to investigate myself little later, but maybe u have an idea what's the bug

@@ -0,0 +1,34 @@
import {colorsPalette} from '../src/style/colorsPalette';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you create this? should the modifier be auto generated?

Copy link
Contributor Author

@Niryo Niryo Mar 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually you are right. initially, this file contained lots of manual types with all the combination of the dynamic usage, for example "paddingT-s3". as you can see, the problem is with the s3, we can't automatically generate typings for the dynamic value, so the former guy that created the manual types, added predefined values for many of the dynamic ones and I wanted to use them. but then I found out that typescript can't read the '-' symbol, so it didn't help us and I removed it.
If you want we can add those types to the modifiers.ts file itself, it's up to you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we will never support dynamic modifiers values (for instance if someone load his own colors or spacings values)
So maybe it's ok not to have typings for that.. I don't know.
the question is, if user will use the component with a modifier that has no typing
<View bg-customColor .../>
will they get a tslint error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I say dynamic, I mean the value. when you type paddingT-785, the value is dynamic and typescript currently don't have support for patterns, so we can't verify that the value is good. as a workaround, the dude from Kiev added some typings for common values, like padding-5 | padding-6 | padding-7| etc... but it doesn't work, because as I said, it looks like typescript can't handle the -.

@Niryo
Copy link
Contributor Author

Niryo commented Mar 22, 2020

@ethanshar oh, I'll check it too and update

@ethanshar ethanshar merged commit 7b5eb1b into master Mar 23, 2020
@ethanshar ethanshar deleted the converting-view-to-typescript branch July 26, 2020 05:41
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.

2 participants