-
Notifications
You must be signed in to change notification settings - Fork 138
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
Generate TypeScript ical.d.ts from jsdoc and include in releases #367
Comments
I'm mostly worried about keeping this file up to date. @glixlur Did you write that by hand, or was it automatically generated? |
Cool - tx @glixlur |
hi, how can I use @glixlur types? |
Hi Just to understand. in:
because, as far as I can see it seems optional component.js
|
I made it work using undefined, you should be able to make it right within the d.ts file though. |
I have a manually created (though incomplete) typing file I created. I use it extensively in and add parts as I get to them https://github.com/etesync/etesync-web The actual file: https://github.com/etesync/etesync-web/blob/master/src/types/ical.js.d.ts (I updated it a bit more since, will push those local changes soon I hope). I can take a look into making it more complete and submit a patch if there's interest. @kewisch ? Otherwise it can go to https://github.com/DefinitelyTyped/DefinitelyTyped but then you'd have even less control when it comes to keeping it up to date. |
I'd prefer something that is auto-generated, I don't want to be updating this file manually or requiring it for people making pull requests. |
Obviously preferable, though how often does the API actually change? At the moment people (like myself and @Proteria) end up manually implementing it for ourselves, and who knows how many others like us are there. I think having a version (@Proteria's?) that covers what there's there now and worst case have her lag behind a bit in the future or until an automatic generator is implemented may be better, no? I'm content with my hand-rolled types for myself, so no rush on my side. I'm just giving feedback based on my usage of this library and others with missing or incomplete typings. |
up! I'm just writing a new project and I don't really like to take half a day fixing other types or writing it myself. |
@HugoGresse, it's far from complete, but take a look at: https://github.com/etesync/ios/blob/master/src/types/ical.js.d.ts |
thanks @tasn 👍 |
@HugoGresse, tslint is deprecated, I use eslint with the typescript plugin (you can see the config in the root of the repo). |
arf, Firebase functions still use it :( |
No idea then, not important enough to try to figure out the tslint rule. Thanks though. |
Are there any official typings available yet for ical.js? |
Up |
Would you mind if I use your definitions in the meantime? @tasn |
Careful using that .d.ts file - the top of it states it is GPL3 meaning your entire codebase becomes GPL if you include it. |
@tasn any chance you'd be able to make a copy with the same license as this codebase? |
@zomars, sorry, sure thing! @MattRiddell, sure thing, sorry about that, will do now. |
Done. We actually have two versions, not sure which is newer/better: https://github.com/etesync/ios/blob/master/src/types/ical.js.d.ts Relicensed both. |
@tasn awesome, thanks man! |
In light of new recent comments I took a quick look on the net, apparently tsc can create the declaration file based on jsdoc? ical.js uses fairly comprehensive jsdoc, so maybe auto generated types are not too far off. If someone wants to see how well these compare to the manual version that would be pretty cool. Note for the record I have a draft pr for es6 modules support which changes some doc patterns, so maybe base any changes off of that. |
I had a look at auto generating typescript definitions from jsdocs. Overall it worked well, but typescript stumbled on the Side note on es modules, I added a re-export of items from the default namespace as named exports. This means you can import classes directly, and get the advantage of tree-shaking like: import { parse, Event } from 'ical.js'; Here if a diff of what I did: es6...joelshepherd:es6 |
@kewisch https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html, could set up either CI or a commit hook to update the types automatically. |
Related: I forked the project and ported the entire project to TypeScript. I’m open to merging this to upstream if anyone is interested. |
I'll happily migrate if we get passing tests and full coverage @alex-kinokon 🙏🏽 |
slightly urgent considering the entire ecosystem is going to ts. Can't run this at all in post 0.71 react-native projects |
I’m fairly certain you can just put |
You mentioned in #597 that you'd prefer not to write a separate d.ts file but generate it from jsdoc which is definitely possible with const data = "...some ical data";
const jCalData: any = ICAL.parse(data);
const comp = new ICAL.Component(jCalData, null);
const events = comp.getAllSubcomponents("vevent");
for (const event of events) {
// event is of type ICAL.Component but it has no methods attached
event.addSubcomponent();
} I believe this would have to be fixed in the jsdoc comments as Hopefully this points someone more familiar with this project and jsdoc in the right direction. |
Yes, absolutely happy to take fixes to the docs to support this. I think some of the aliases were used to fix the reference docs generation, I got them from https://jsdoc.app/tags-alias . If there is a better way to do this in TS let's see! @jannikac I know you mentioned you'd prefer someone more familiar to take a look, but maybe you'd be interested in tinkering until it works? It looks like you're already half way in :) |
I took a stab at it and created a PR to discuss this matter further #662. |
I will be stoked when this gets implemented. First the v2 with proper ES6 modules, and then hopefully this. This is even more important due to the potential for confusion with |
Anybody which know if there exists a ical.d.ts file for TypeScript @types module?
The text was updated successfully, but these errors were encountered: