Add TypeScript declarations to @svgr/core
#555
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/gregberge/svgr/5RJ1ijqgdectcLbeyxCVVtjmeDSH |
Codecov Report
@@ Coverage Diff @@
## main #555 +/- ##
=======================================
Coverage 91.39% 91.39%
=======================================
Files 31 31
Lines 1081 1081
Branches 333 333
=======================================
Hits 988 988
Misses 88 88
Partials 5 5 Continue to review full report at Codecov.
|
nirtamir2
changed the title
@svgr/core
| export { default as File } from './File' |
There was a problem hiding this comment.
My IDE complains about it - so I change it.
Is it ok?
| "ci": "yarn build && yarn lint && yarn check-dts && yarn test --ci --coverage && codecov", | ||
| "check-dts": "check-dts", |
There was a problem hiding this comment.
Added check-dts to the ci script
packages/core/src/index.errors.ts
Outdated
| @@ -0,0 +1,10 @@ | |||
| import svgr from './' | |||
There was a problem hiding this comment.
index.d.ts negative test
packages/core/src/index.types.ts
Outdated
| @@ -0,0 +1,15 @@ | |||
| import svgr from './' | |||
There was a problem hiding this comment.
index.d.ts positive test
| @@ -0,0 +1,10 @@ | |||
| import svgr from './' | |||
|
|
|||
| // THROWS Argument of type 'null' is not assignable to parameter of type 'string'. | |||
There was a problem hiding this comment.
The TS compiler error message
|
Can this be merged yet? |
|
I'm not this project maintainer, but from my side this PR is ready. I added the comments to make the review easier |
| } | ||
|
|
||
| type ConvertT = { | ||
| (svgCode: string, opts?: SvgrOpts, state?: TemplateData): Promise<void> |
There was a problem hiding this comment.
I'm still spinning up on svgr, but isn't a Promise<string> returned?
|
|
||
| type ConvertT = { | ||
| (svgCode: string, opts?: SvgrOpts, state?: TemplateData): Promise<void> | ||
| sync: (svgCode: string, opts?: SvgrOpts, state?: TemplateData) => void |
There was a problem hiding this comment.
I believe this sync function returns string with svg code, instead of void.
| sync: (svgCode: string, opts?: SvgrOpts, state?: TemplateData) => void | |
| sync: (svgCode: string, opts?: SvgrOpts, state?: TemplateData) => string |
|
|
||
| export interface SvgrOpts { | ||
| /** Specify a custom config file. */ | ||
| configFile?: string |
There was a problem hiding this comment.
I think this object can also have the plugins attribute
/** Specify custom plugins installed. */
plugins?: string[];
gregberge
force-pushed
the
add-typescript-declarations
branch
from
715b3b4 to
12cb449
Compare
|
Thanks, will try to rewrite everything in TypeScript, but I think I will make a release before that includes typings. |
Summary
Following #513 I added TypeScript declaration file (
index.d.ts) to@svgr/corewith @jschaf #513 (comment) modified a little.This will help for easier auto-completion and for other library authors that use TypeScript and want to pass type-safe params to
@svgr/coreNOTICE: as #513 (comment) notes - the types are not 100% compatible but I think for most people it will be good enough (and it can improve it over time).
Test plan
I use https://github.com/ai/check-dts for checking
.d.tsfilesindex.types.tsshows positive testsindex.errors.tsincludes improper usage of the libraryI added it to the CI too.