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

add type definition #490

Merged
merged 2 commits into from
Mar 5, 2018
Merged

add type definition #490

merged 2 commits into from
Mar 5, 2018

Conversation

taoqf
Copy link
Contributor

@taoqf taoqf commented Feb 3, 2018

No description provided.

@Rycochet
Copy link
Contributor

Rycochet commented Feb 3, 2018

Should reference to #238 and #177 here, and maybe compare contents of my gist linked in it.

@taoqf
Copy link
Contributor Author

taoqf commented Feb 5, 2018

I just want to help, and I am using this project in my work.

@kachkaev
Copy link

kachkaev commented Feb 20, 2018

Having typings for exceljs would be awesome! @guyonroche 🙏

package.json Outdated
@@ -98,4 +99,4 @@
"LICENSE",
"README.md"
]
}
}

Choose a reason for hiding this comment

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

ouch 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the formatter

@guyonroche guyonroche merged commit 46853d1 into exceljs:master Mar 5, 2018
@guyonroche
Copy link
Collaborator

Thanks @taoqf for doing this!
I'm not the hugest fan of types in JS but I know a lot of people are :-) so this will be a great addition

@Purus
Copy link

Purus commented Mar 22, 2018

How should I use this typings for my Typescript application?

@taoqf
Copy link
Contributor Author

taoqf commented Mar 22, 2018

Just install this lib will be OK, after new version if published.

@Purus
Copy link

Purus commented Mar 22, 2018

Any idea when this will be published? Trying to use in a project.

@taoqf
Copy link
Contributor Author

taoqf commented Mar 22, 2018

ask @guyonroche

@ahaverty
Copy link

@taoqf @guyonroche It looks like this is in the repo and released 👏

But on v1.0.1 import * as ExcelJs from 'exceljs'; isn't giving me any types as expected.
I can't find a index.d.ts in the dist after installing either
Has anyone gotten it to work? 🤔

@taoqf
Copy link
Contributor Author

taoqf commented Mar 28, 2018

@guyonroche , please put index.d.ts in release.

@grbspltt
Copy link

@ahaverty I found the typings in the repo, copied and pasted into the node_modules/exceljs folder. Not a great solution but works until the release is fixed.

@spacem
Copy link

spacem commented Jun 26, 2018

Before this change I was using types from https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/exceljs
ie. installed with npm install @types/exceljs

Now I updated to the latest exceljs and the 'official' types from this merge don't seem so good. To give one example the font interface says that font requires all the parameters where I believe actually they should be optional?

@kachkaev
Copy link

@spacem could you please submit a PR that would fix the typing issues you are facing?

@taoqf taoqf mentioned this pull request Jun 26, 2018
@taoqf
Copy link
Contributor Author

taoqf commented Jun 26, 2018

@spacem anything else?

@spacem
Copy link

spacem commented Jun 26, 2018

Beat me to the pr.. Maybe others should be partial but I am not using those.
The only other compile error I get now is from streams. The DefinitelyTyped types had import { Writable, Stream } from 'stream'; so could pass the result of createInputStream() into stream.pipe()

Thanks guys!

@taoqf
Copy link
Contributor Author

taoqf commented Jun 27, 2018

@spacem Maybe you should install @types/node for now.
npm install @types/node

I've add this into dependencies.
ad3b792

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.

8 participants