-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Add @theme-ui/components #41057
Add @theme-ui/components #41057
Conversation
@hasparus Thank you for submitting this PR! Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes. In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day! |
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? These typings are for a package that doesn’t yet exist on master, so I don’t have anything to compare against yet! In the future, I’ll be able to compare PRs to theme-ui__components with its source on master. Comparison details 📊
|
I'm failing
It looks like all of the props are required during the dtslint check.
|
looks good 👍 |
We've gotten sign-off from a reviewer 👏. A maintainer will soon review this PR and merge it if there are no issues. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for contributing to DefinitelyTyped! |
Co-Authored-By: Petr Bela <bela.petr@gmail.com>
@hasparus I think we should also define and export |
hey @petrbela 👋 I omitted it because it's the same thing as BoxProps (modulo augmentation) but I think it should be exported anyway We could rename FlexStyleProps to FlexOwnProps and do export interface FlexProps extends Assign<React.ComponentProps<'div'>, FlexOwnProps> {} so it can be augmented in user code before adding to DT if theme-ui Flex gets more props |
Please fill in this template.
npm test
.)Failed on my machine with
npm run lint package-name
(ortsc
if notslint.json
is present).Select one of these and delete the others:
If adding a new definition:
.d.ts
files generated via--declaration
dts-gen --dt
, not by basing it on an existing project.I didn't find the docs on how to run dts-gen for namespaced package.
The latter command generated a proper DT directory and I started from that.
tslint.json
should be present and it shouldn't have any additional or disabling of rules. Just content as{ "extends": "dtslint/dt.json" }
. If for reason the some rule need to be disabled, disable it for that line using// tslint:disable-next-line [ruleName]
and not for whole package so that the need for disabling can be reviewed.tsconfig.json
should havenoImplicitAny
,noImplicitThis
,strictNullChecks
, andstrictFunctionTypes
set totrue
.