Add voilab-pdf-table by danielb7390 · Pull Request #54143 · DefinitelyTyped/DefinitelyTyped

danielb7390 · 2021-06-28T18:51:08Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

Adding a new definition:

The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
If this is for an npm package, match the name. If not, do not conflict with the name of an npm package.
Create it with dts-gen --dt, not by basing it on an existing project.
Represents shape of module/library correctly
tslint.json should contain { "extends": "dtslint/dt.json" }, and no additional rules.
tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

This one is quite more complex than my first package that i did.
I did my best, but I'm no typescript expert! I'm open to suggestions!

Used 2x no-misused-new. the suggestion is to use a class but i don't think it makes sense in both situations?
Used 2x no-unnecessary-generics. I'm not sure about this one, i get it T is not used in the arguments, but i want to have a specific return type?

typescript-bot · 2021-06-28T18:51:42Z

@danielb7390 Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

voilab-pdf-table (new!) — on npm, on unpkg
- 1 added owner: ✎@danielb7390

Code Reviews

This PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Only a DT maintainer can approve changes when there are new packages added

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 54143,
  "author": "danielb7390",
  "headCommitOid": "bea8ac69fa929cd57f36f6bab6602b167f55e83b",
  "lastPushDate": "2021-07-01T17:37:02.000Z",
  "lastActivityDate": "2021-07-01T20:25:07.000Z",
  "mergeOfferDate": "2021-07-01T20:19:56.000Z",
  "mergeRequestDate": "2021-07-01T20:25:07.000Z",
  "mergeRequestUser": "danielb7390",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "voilab-pdf-table",
      "kind": "add",
      "files": [
        {
          "path": "types/voilab-pdf-table/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/voilab-pdf-table/plugins/fitcolumn.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/voilab-pdf-table/plugins/rowshader.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/voilab-pdf-table/tsconfig.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/voilab-pdf-table/tslint.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/voilab-pdf-table/voilab-pdf-table-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [],
      "addedOwners": [
        "danielb7390"
      ],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2021-07-01T20:19:21.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "DanielRosenwasser",
      "date": "2021-07-01T08:07:36.000Z",
      "abbrOid": "51aa511"
    }
  ],
  "mainBotCommentID": 869934423,
  "ciResult": "pass"
}

typescript-bot · 2021-06-28T18:51:44Z

🔔 @danielb7390 — you're the only owner, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)

typescript-bot · 2021-06-28T19:00:12Z

@danielb7390 The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

types/voilab-pdf-table/index.d.ts

peterblazejewicz · 2021-06-28T20:54:46Z

types/voilab-pdf-table/index.d.ts

+export = VoilabPdfTable;
+export as namespace VoilabPdfTable;
+
+declare class VoilabPdfTable<T = any> {


Is the any the best choice here? It looks like non-base data type IMO

Can you explain better?

We would like to avoid people doing simple typo kind of thing with literals for e.g. number? this could be unknown (as per handbook) or something that reassemble the type used (note: I've not used this native package, but I understand it accepts structure objects, so Record<string, Type> where Typeare allowed types union). Instead of Record one could use interface Type { [key: string]: SupportedType };.
What I meant: would be nice to guide what's allowed, but that's ok to use unkown (as per handbook)

Suggested change

declare class VoilabPdfTable<T = any> {

interface VoilabPdfTableDefaultType { [key: string]: string | number}

declare class VoilabPdfTable<T = VoilabPdfTableDefaultType > {

If i understand you correctly, you are suggesting something like this?
Should i also be checking that whatever type the user passes, the object values are always string|number like so:

Suggested change

declare class VoilabPdfTable<T = any> {

interface VoilabPdfTableDefaultType { [key: string]: string | number}

declare class VoilabPdfTable<T extends VoilabPdfTableDefaultType = VoilabPdfTableDefaultType > {

I apologize if my questions are a bit weird, but i'm still learning TS

Yes, if it make sense, I'll approve anyway. You can modify this further

typescript-bot · 2021-06-28T21:32:43Z

@peterblazejewicz Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

peterblazejewicz

LGTM!

typescript-bot · 2021-07-01T07:06:04Z

@peterblazejewicz Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

DanielRosenwasser · 2021-07-01T08:07:36Z

types/voilab-pdf-table/index.d.ts

+// Definitions by: Daniel Sousa <https://github.com/danielb7390>
+// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
+
+/// <reference types="pdfkit" />


Instead of a /// <reference can you get away with a simple import?

typescript-bot · 2021-07-01T17:42:29Z

@DanielRosenwasser, @peterblazejewicz Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

typescript-bot · 2021-07-01T20:19:56Z

@danielb7390 Everything looks good here. Great job! I am ready to merge this PR (at bea8ac6) on your behalf.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

typescript-bot · 2021-07-01T20:19:57Z

@DanielRosenwasser Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

danielb7390 · 2021-07-01T20:25:07Z

Ready to merge

typescript-bot · 2021-07-01T20:33:07Z

I just published @types/voilab-pdf-table@0.5.0 to npm.

danielb7390 added 2 commits June 28, 2021 19:30

Add types for voilab-pdf-table

29aac00

Prettier

a7551e0

typescript-bot added the New Definition This PR creates a new definition package. label Jun 28, 2021

Add OTHER_FILES

c8c3454

typescript-bot added Check Config Changes a module config files The CI failed When GH Actions fails labels Jun 28, 2021

OTHER_FILES not needed after all

04001dd

typescript-bot removed Check Config Changes a module config files The CI failed When GH Actions fails labels Jun 28, 2021

danielb7390 changed the title ~~voilab-pdf-table~~ Add voilab-pdf-table Jun 28, 2021

peterblazejewicz reviewed Jun 28, 2021

View reviewed changes

types/voilab-pdf-table/index.d.ts Outdated Show resolved Hide resolved

peterblazejewicz reviewed Jun 28, 2021

View reviewed changes

Remove not needed export as namespace

334a6ef

peterblazejewicz approved these changes Jun 30, 2021

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jun 30, 2021

Add default type

51aa511

typescript-bot removed Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jul 1, 2021

DanielRosenwasser reviewed Jul 1, 2021

View reviewed changes

Replace reference with import

bea8ac6

peterblazejewicz approved these changes Jul 1, 2021

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jul 1, 2021

typescript-bot merged commit 3e2f2ee into DefinitelyTyped:master Jul 1, 2021

	declare class VoilabPdfTable<T = any> {
	interface VoilabPdfTableDefaultType { [key: string]: string \| number}
	declare class VoilabPdfTable<T = VoilabPdfTableDefaultType > {

	declare class VoilabPdfTable<T = any> {
	interface VoilabPdfTableDefaultType { [key: string]: string \| number}
	declare class VoilabPdfTable<T extends VoilabPdfTableDefaultType = VoilabPdfTableDefaultType > {

Conversation

danielb7390 commented Jun 28, 2021

Uh oh!

typescript-bot commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1 package in this PR

Code Reviews

Status

Uh oh!

typescript-bot commented Jun 28, 2021

Uh oh!

typescript-bot commented Jun 28, 2021

Uh oh!

Uh oh!

peterblazejewicz Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

danielb7390 Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

peterblazejewicz Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielb7390 Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterblazejewicz Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

typescript-bot commented Jun 28, 2021

Uh oh!

peterblazejewicz left a comment

Choose a reason for hiding this comment

Uh oh!

typescript-bot commented Jul 1, 2021

Uh oh!

DanielRosenwasser Jul 1, 2021

Choose a reason for hiding this comment

Uh oh!

danielb7390 Jul 1, 2021

Choose a reason for hiding this comment

Uh oh!

typescript-bot commented Jul 1, 2021

Uh oh!

typescript-bot commented Jul 1, 2021

Uh oh!

typescript-bot commented Jul 1, 2021

Uh oh!

danielb7390 commented Jul 1, 2021

Uh oh!

typescript-bot commented Jul 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

typescript-bot commented Jun 28, 2021 •

edited

Loading

peterblazejewicz Jun 28, 2021 •

edited

Loading

danielb7390 Jun 28, 2021 •

edited

Loading