Skip to content
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

TypeScript and definitions. #51

Closed
danno-ax opened this issue Aug 29, 2017 · 23 comments
Closed

TypeScript and definitions. #51

danno-ax opened this issue Aug 29, 2017 · 23 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@danno-ax
Copy link

Im wondering if typings could be added/maintained for this project? Iv got a patch almost ready to go in my own repo i could add it?
Would it be too bold of me to even propose a conversion to TS?
bitmoji

@wojtekmaj wojtekmaj added the question Further information is requested label Aug 29, 2017
@wojtekmaj
Copy link
Owner

wojtekmaj commented Aug 29, 2017

Hey! Thanks for asking.
I do not plan to convert the project to TS. I feel like that would slow my development down significantly while not giving much in return; the most vital elements of the project are React components, which are equipped with propTypes which works just fine for my needs. Maybe sometime in the future I'd add Flow or something similar, but I have no plans set up for this.
I'd also want to remind that this project is licensed on MIT license, so you are free to do absolutely anything you would like to do with it, including creating competing versions based on it ;)
Is there any specific problem you would like to solve by migration to TS?

@danno-ax
Copy link
Author

Hey! No prob! I don't mean to be that guy.
I guess considering this project isn't too complex at the moment it should be fine not having any type system. There isn't a single specific issue that TS or Flow solves, its that it solves a set of issues common to large applications, which I won't go into rn.
Anyway I think I can just PR my type definitions to definitely typed so all good :)

@alex-sherwin
Copy link

alex-sherwin commented Sep 11, 2017

Adding typings does not require you to convert any JavaScript to TypeScript

Providing typings is just additional metadata that is fed into the TypeScript compiler so that people who write projects in TypeScript can consume your JavaScript library with strong typing guarantees

To provide these in your project, you would add .d.ts (TypeScript definitions) to your project and include them in your published npm package, which describe the structure of your JavaScript models/classes/functions etc.

More and more people are starting new projects as TypeScript and only consuming libraries which expose TypeScript type information, and I can attest, it makes web development so much smoother. I'd like to use this project, but a lack of typings prevents me from doing so.

If none of the other alternative PDF.js based React components don't either, I'll probably bite the bullet and create typings and contribute them back

@wojtekmaj
Copy link
Owner

wojtekmaj commented Sep 11, 2017

Hmmmm, I don't know if I feel confident enough to do it right now, but it is indeed an interesting idea, so I'm going to keep this issue open in case me or anyone else would like to have some fun with it!

@wojtekmaj wojtekmaj reopened this Sep 11, 2017
@wojtekmaj wojtekmaj added the help wanted Extra attention is needed label Sep 11, 2017
@wojtekmaj wojtekmaj added this to the Future milestone Sep 11, 2017
@wojtekmaj wojtekmaj added enhancement New feature or request and removed question Further information is requested labels Sep 11, 2017
@dimon82
Copy link

dimon82 commented Nov 23, 2017

@alex-sherwin did you get a chance to create typings for this package? If so, can you maybe share them or issue a PR maybe?

@alex-sherwin
Copy link

Hi @dimon82

I did not end up using this project at all. I used npm packages pdfjs-dist with typings from@types/pdfjs-dist to directly depend on PDF.js and derived some custom React components to suite my use cases.

@A4Abhiraj
Copy link

Anyone still stuck at the transpiler errors, add this as a bare minimum. You won't get any intellisense support.

@types
|- react-pdf
|-index.d.ts

import * as React from 'react';

export class Document extends React.Component<any,any>{}

export class Page extends React.Component<any,any>{}

@saadq
Copy link
Contributor

saadq commented Feb 1, 2018

For those who are using Flow, I published react-pdf libdefs for v2 on flow-typed. Some types could use improvement, but it's pretty complete.

@chandrajob365
Copy link

@danno-ax : Can u please share/publish your typescript definition for react-pdf.

@saadq
Copy link
Contributor

saadq commented Mar 6, 2018

I've started learning some TypeScript as well... once v3 is out of alpha/beta, I can look into updating the Flow definitions as well as creating some TypeScript definitions.

@mindblight
Copy link

mindblight commented May 5, 2018

I threw together some basic TS typings that I'm using on my project. They're not well tested, but they could serve as a starting point:. This is for react-pdf 3.0.5

//@types/react-pdf/index.d.ts
declare module 'react-pdf' {
  import * as React from 'react';

  interface CommonPdfProps {
    error?: React.ReactNode | Function;
    // TODO: Change to real ref type
    inputRef?: (ref: any) => void;
    loading?: React.ReactNode | Function;
    noData?: React.ReactNode | Function;
    onLoadError?: (error: Error) => void;
    onLoadSuccess?: (pdf: Pdf) => void;

  }

  interface Pdf {
    numPages: number;
  }

  interface DocumentProps extends CommonPdfProps {
    file: string | File;
    
    onItemClick?: (pageNumber: number) => void;
    onSourceError?: (error: Error) => void;
    rotate?: 0 | 90 | 180 | 270;

  }
  class Document extends React.Component<DocumentProps>{}


  interface PageProps extends CommonPdfProps {
    customTextRenderer?: (args: { str: string, itemIndex: number }) => React.ReactNode;
    onRenderError?: (error: Error) => void;
    onRenderSuccess?: () => void;
    // TODO: Add array typings
    onGetAnnotationsSuccess?: (annotations: any[]) => void;
    onGetAnnotationsError?: (error: Error) => void;
    // TODO: Add array typings
    onGetTextSuccess?: (items: any[]) => void;
    onGetTextError?: (error: Error) => void;

    pageIndex?: number;
    pageNumber?: number;
    renderAnnotations?: boolean;
    renderTextLayer?: boolean;
    rotate?: 0 | 90 | 180 | 270;
    scale?: number;
    width?: number;
  }
  class Page extends React.Component<PageProps>{}


  interface OutlineProps {
    onItemClick?: (pageNumber: number) => void;
    onLoadError?: (error: Error) => void;
    onLoadSuccess?: (pdf: Pdf) => void;
    onParseError?: (error: Error) => void;
    // TODO: Add array typings
    onParseSuccess?: (outline: any[]) => void;
  }
  class Outline extends React.Component<OutlineProps>{}  
}

It would be great to include some typings in the project. Material UI is a good example of coding in JS but including TS definitions: https://github.com/mui-org/material-ui/tree/v1-beta/packages/material-ui/src

UPDATE I changed declare module 'react-pdf' to declare module 'react-pdf/dist/entry.webpack' to get webpack and workers to work properly

@amogower
Copy link

amogower commented Nov 14, 2018

I threw together some basic TS typings that I'm using on my project. They're not well tested, but they could serve as a starting point:. This is for react-pdf 3.0.5

//@types/react-pdf/index.d.ts
declare module 'react-pdf' {
  import * as React from 'react';

  interface CommonPdfProps {
    error?: React.ReactNode | Function;
    // TODO: Change to real ref type
    inputRef?: (ref: any) => void;
    loading?: React.ReactNode | Function;
    noData?: React.ReactNode | Function;
    onLoadError?: (error: Error) => void;
    onLoadSuccess?: (pdf: Pdf) => void;

  }

  interface Pdf {
    numPages: number;
  }

  interface DocumentProps extends CommonPdfProps {
    file: string | File;
    
    onItemClick?: (pageNumber: number) => void;
    onSourceError?: (error: Error) => void;
    rotate?: 0 | 90 | 180 | 270;

  }
  class Document extends React.Component<DocumentProps>{}


  interface PageProps extends CommonPdfProps {
    customTextRenderer?: (args: { str: string, itemIndex: number }) => React.ReactNode;
    onRenderError?: (error: Error) => void;
    onRenderSuccess?: () => void;
    // TODO: Add array typings
    onGetAnnotationsSuccess?: (annotations: any[]) => void;
    onGetAnnotationsError?: (error: Error) => void;
    // TODO: Add array typings
    onGetTextSuccess?: (items: any[]) => void;
    onGetTextError?: (error: Error) => void;

    pageIndex?: number;
    pageNumber?: number;
    renderAnnotations?: boolean;
    renderTextLayer?: boolean;
    rotate?: 0 | 90 | 180 | 270;
    scale?: number;
    width?: number;
  }
  class Page extends React.Component<PageProps>{}


  interface OutlineProps {
    onItemClick?: (pageNumber: number) => void;
    onLoadError?: (error: Error) => void;
    onLoadSuccess?: (pdf: Pdf) => void;
    onParseError?: (error: Error) => void;
    // TODO: Add array typings
    onParseSuccess?: (outline: any[]) => void;
  }
  class Outline extends React.Component<OutlineProps>{}  
}

It would be great to include some typings in the project. Material UI is a good example of coding in JS but including TS definitions: https://github.com/mui-org/material-ui/tree/v1-beta/packages/material-ui/src

UPDATE I changed declare module 'react-pdf' to declare module 'react-pdf/dist/entry.webpack' to get webpack and workers to work properly

You're most likely better off leaving it as declare module 'react-pdf' and adding a "react-pdf": ["react-pdf/dist/entry.webpack"] to the paths object in your tsconfig.json and 'react-pdf': 'react-pdf/dist/entry.webpack' to the alias object in your webpack config.

But nice work with the declaration!

@moffsugita
Copy link

@alex-sherwin pdfjs is better than this?

@moffsugita
Copy link

moffsugita commented Nov 30, 2018

@saadq I tried. but error...

> flow-typed install react-pdf
UNCAUGHT ERROR: Error: Failed to find a flow-bin dependency in package.json.
Please install flow-bin: `npm install --save-dev flow-bin`
    at findFlowSpecificVer$ (/usr/local/lib/node_modules/flow-typed/dist/lib/npm/npmProjectUtils.js:245:17)
    at tryCatch (/usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:117:21)
    at tryCatch (/usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:65:40)
    at invoke (/usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:155:20)
    at /usr/local/lib/node_modules/flow-typed/node_modules/regenerator-runtime/runtime.js:165:13
    at <anonymous>
> npm install --save-dev flow-bin
npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN ts-jest@22.0.1 requires a peer of jest@^22.0.1 || ^22.1.0-alpha.1 || ^23.0.0-alpha.1 but none is installed. You must install peer dependencies yourself.
npm WARN The package ts-jest is included as both a dev and production dependency.

+ flow-bin@0.87.0
> flow-typed install react-pdf
Error: Unable to find a flow project in the current dir or any of it's parent dirs!
Please run this command from within a Flow project.

@moffsugita
Copy link

@mindblight

could you append them in the file?

  interface GlobalWorkerOptionsProps {
    workerSrc: string;
  }

  interface PDFjsStatic {
    version: string;
    GlobalWorkerOptions: GlobalWorkerOptionsProps;
  }

  let pdfjs: PDFjsStatic;

@wojtekmaj

Will you do pull-request to DefinitelyTyped/DefinitelyTyped?

@saadq
Copy link
Contributor

saadq commented Dec 3, 2018

Hey @moffsugita, in regards to this message:

flow-typed install react-pdf
Error: Unable to find a flow project in the current dir or any of it's parent dirs!
Please run this command from within a Flow project.

This usually means that the flow project you are currently in is missing a .flowconfig file. Can you double check to make sure you have it there?

@wojtekmaj
Copy link
Owner

@moffsugita I will not since I don't feel like an expert in that matter. As I said, I welcome all PRs though, so if anyone would like to add typings to this repo, I will certainly appreciate it.

moffsugita added a commit to moffsugita/DefinitelyTyped that referenced this issue Dec 6, 2018
@moffsugita
Copy link

Hello @saadq

I created .flowconfig. but I didn't understand how to install your type files.

npx flow init

edited .flowconfig file.

[ignore]
.*/node_modules/.*

[include]

[libs]
./src/decls

[lints]

[options]

[strict]

ran flow-typed install react-pdf

• Searching for 1 libdefs...
• rebasing flow-typed cache...
!! No flow@v0.87.0-compatible libdefs found in flow-typed for the explicitly requested libdefs. !!

Consider using `flow-typed create-stub react-pdf` to generate an empty libdef that you can fill in.

flow-typed create-stub react-pdf

• Creating 1 stub...
  • react-pdf@4.0.0
    └> flow-typed/npm/react-pdf_vx.x.x.js

@moffsugita
Copy link

moffsugita commented Dec 7, 2018

@saadq Perhaps. I had should append your module's version when run install command like below.

flow-typed install react-pdf@2.1.x

it pulled to flow-typed/npm/react-pdf_v2.1.x.js from your type file.

however... my project (create-react-app created) didn't recognize the file yet.

(5,44): Could not find a declaration file for module 'react-pdf'. '/path/to/node_modules/react-pdf/dist/entry.js' implicitly has an 'any' type.
  Try `npm install @types/react-pdf` if it exists or add a new declaration (.d.ts) file containing `declare module 'react-pdf';`

@moffsugita
Copy link

@saadq perhaps..
flow is just for JavaScript? just checking type?
I just wanted to use react-pdf for TypeScript.

@moffsugita
Copy link

Finally. It is my good practice which I thought before releasing type file in DefinitelyTyped/DefinitelyTyped repository.

I created files in the project.

> tree workspace/
workspace/
└── react-pdf
    ├── index.d.ts
    └── package.json

index.d.ts is from @mindblight 's

package.json is like below. It is temporary so 0.0.1

{
  "version": "0.0.1"
}

@saadq
Copy link
Contributor

saadq commented Dec 7, 2018

@moffsugita So both Flow and TypeScript are two separate typecheckers people use for projects. TypeScript is both the type checker and compiler while Flow is just the type checker (babel is used to do the compilation).

So the problem you ran into with Flow is that flow-typed currently only has types for v2 of react-pdf, I hadn't gotten around to doing v3 so there aren't types for it as of right now. Glad you got it sorted out though.

@CodeDaraW
Copy link

Hi all, I have just contributed @types/react-pdf to DefinitelyTyped, see Add types for react-pdf by CodeDaraW · Pull Request #35317 · DefinitelyTyped/DefinitelyTyped. So we can use @types/react-pdf now.

alexandernanberg pushed a commit to alexandernanberg/react-pdf that referenced this issue Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests