Typescript

[sonarcloud[bot]]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[YizYah]

@mjradwin This pr appears to me to succeed at generating all of the TS types automatically rather than manually.

• As you can see in package.json, the manual hebcal.d.ts is replaced by ./dist/index.d.ts
• I added the packages typescript and tslib. Also, @rollup/plugin-typescript
• I added the config file tsconfig.com. Currently, I left all of the commented out options in the file. I usually delete those, but I thought you might be interested to see them.
• fundamentally, the only change is to rename index.js to be index.ts. Then, I had to update that name in the rollup.config.js file. I also added the plugin where needed there.
• in hebcal.ts I had to change two things in your jsdoc directives. We had to make the CalOptions really optional, and to have the Event type be your Event rather than the js Event type.

In addition, I took two liberties:

• I added --production to the build to stop distributing the devDependencies
• I added HolidayDesc, an enum that serves the same goal as holidayDisc. HOWEVER: please note that I have not yet removed your holidayDesc. The reason is that a js file cannot import a ts file (although the opposite is fine). Since holiday.js uses staticHolidays.js, I did not want to convert both to ts in this PR.

The update can be done one file at a time, but it must be done top-down. So, again, we will need to create holiday.ts before staticHolidays.js.

The tests are, of course, all passing. And I confirmed that I could import EventCalendar and Event without difficulty. But, I have not confirmed that other types are imported without a problem. Normally, this wouldn't be a concern. It's just that you are using the jsdoc directives, which are influencing tsc. I don't feel confident that I know all of your objects, so I can't be sure. It should be very easy to check, because any issues will be static. So you can just import into a "hello world" ts file and see what you get. But if there were a problem, someone would probably raise an issue almost immediately. I'm not sure what to advise for that.

[YizYah]

@mjradwin I wanted to change this to draft, don't think I can now that I've created it. The only issue is the final paragraph in the last comment. I am actually linked to my local version of this branch, and it's working well.

I just thought of a very simple way to check this--look through the manual hebcal.d.ts and confirm that everything in it appears correctly in the generated d.ts files in dist. I don't have time today. Could do it Sunday if you'd like.

[YizYah]

fixes #442

[mjradwin]

Reviewing. One quick question: is there a reason why the tslib in package.json is listed in dependencies and not devDependencies?

[YizYah]

I'm not sure. It could be that we could move it to devdependencies, because the importHelpers flag is not set. I added it because I saw a claim that it was needed to get the rollup working. We could try removing it or moving it.