TypeScript dependencies

[Manc]

Thanks for the 2.0.0 update! I'm just trying to upgrade my project to make use of the new version, but when I build my project with tsc, I get the following errors:

node_modules/ical-generator/dist/event.d.ts:1:28 - error TS2307: Cannot find module 'rrule' or its corresponding type declarations.

1 import type { RRule } from 'rrule';
                             ~~~~~~~

node_modules/ical-generator/dist/types.d.ts:3:28 - error TS2307: Cannot find module 'dayjs' or its corresponding type declarations.

3 import type { Dayjs } from 'dayjs';
                             ~~~~~~~

It looks like the included .d.ts files import packages like rrule, dayjs, moment, luxon and more, but ical-generator has them as dev dependencies only, and to my best knowledge, this is not the correct way. If ical-generator wants to support third-party packages, it must reference them as proper dependencies in package.json, not just devDependencies.

The reason, why only those two (rrule and dayjs) are missing in my project is, I require luxon in myself as well as another package, that requires cron-parser, that requires moment-timezone, that requires moment.

Personally, I think it would make sense to give up support for several third-party date utilities and reduce input dates to native Date or JavaScript native timestamps in milliseconds (as in Date.now()). If you really want to support other date utilities, you must also bind support to specific versions of those as any of those could change their API at any time.

I suppose my possible workarounds for now would be to either require other utilities that I don't really need in my project, or to revert back to version 1.x. If you have any tips and tricks, please let me know. Thanks!

[Manc]

Sorry, I see you already declared said dependencies as peerDependencies, but to be honest, I don't have experience with that, but it doesn't seem to work for me.

[sebbo2002]

The dependencies required for Typescript are not only stored in the devDependencies, but also in the peerDependencies provided for this purpose, as you can check here. These dependencies should be installed automatically as long as you are using npm > v6, so please check if you are using a current npm version (see npm 7 release note).

This library actually only uses import type imports from these third party libraries everywhere, so the size of the compilation doesn't change at all. This feature is available from Typescript 3.8 onwards. The third party libraries are not included in the final product and are only necessary for Typescript to check whether the types are compatible. I think this is acceptable.

Therefore, I would disagree at this point and say that the support of several libraries is feasible at this point, since the libraries must be installed for Typescript, but they do not have to be used or included in the final product. If you see it differently, feel free to re-open the issue and clarify your point.

[sebbo2002]

Sorry, I see you already declared said dependencies as peerDependencies, but to be honest, I don't have experience with that, but it doesn't seem to work for me.

Sorry, I didn't see your comment when I formulated my previous one. Did you get any further with it?

[Manc]

Thanks for your reply. I'm still investigating. I'm actually using yarn. It's strange, because I agree, peerDependencies sounds like the obvious right choice, but yarn doesn't seem to install those packages in my project. I uninstalled ical-generator and reinstalled it (at first I just upgraded it), but no success yet. I'm checking now if it could be related to my yarn version. TS is on 4.1.x. It's just weird, because my project has actually quite a lot of direct dependencies and a huge amount of indirect ones, but this is the first time I encounter this kind of issue. I'll update this ticket as soon as I find out more.

[Manc]

My Yarn is the most recent 1.x and an upgrade to v2 didn't really work yet. Regardless, I'm feeling confused. It feels somewhat strange to me that npm 7+ is required and it would't work with Yarn 1 at all. As I said, it's the first time I ever encountered an issue like this and I have a feeling that tells me this should be approached slightly differently, but I couldn't figure out how yet.

This question on StackOverflow was the best I could find. The accepted answer is what seems to make sense, but then the second answer also makes sense, but it doesn't seem to work.

I'm also still trying to find another repository/package with a similar problem and a solution for inspiration…

Right now I'm experimenting with a potential solution, I'll let you know, probably tomorrow.

[sebbo2002]

Feel free to let me know if you have a better solution to the problem. The StackOverflow article summarises the possibilities I know quite well, although the former is not practical for me, because with my own types I wouldn't notice if something changed in a third-party library. That's why it was important to me to use the "real" types (as described in the second answer).

I don't use yarn, but maybe there is a parameter or some other way to persuade yarn to install the peerDependencies? Because if I've understood you correctly, the only problem is that yarn doesn't install these dependencies, right?

[Manc]

Last Saturday I continued with my investigation and I already started to write a massive comment with all my findings. I must have closed the tab before I submitted the final comment.

To cut a long story short, I actually went round in circles and didn't find a quick and easy solution. And although it wasn't my intention, I somehow ended up starting to rewrite the library for my own purposes. I'm have just published the fork, in which I have removed support for third-party libraries altogether. I believe, this is not the way you want to go with this library, but feel free to take a look.

I'm open to collaboration. For example, I could imagine that you use my stripped-down library as a base for a future version of your library and extend it with support for third-party libs. In my opinion it would make more sense to work on one base library than on two separate ones.

Anyway, thanks for all the great work you have put into this library so far!

[sebbo2002]

At this point, I hope that yarn will provide support for peerDependencies like npm does from v6 on - because then the problem would have been solved, wouldn't it?

[Manc]

I have doubts that peerDependencies really is the right option for this, but unfortunately I can't substantiate this gut feeling.

For example, popular packages like image minifiers that support optional third-party packages, still do not list them as peerDependencies (see imagemin or next-optimized-images).

An example, where peerDependencies is used, is react-dom, a library that only makes sense to be used in combination with React. React is not optional here, but it's up to the user to add a compatible version to the dependencies.

That's why I think, the question should be: Does the package require lib X for development? If so, add it to devDependencies; if not, don't add it. If it's required, but up to the dev to install it, add it to peerDependencies. I don't know why the decision was made that NPM v7 should install all peer dependencies of all packages and I assume that this decision was made by people who know much more about this stuff than I do, but somehow, it still doesn't feel right. Let's say, my project uses an image minifier, but I only ever need to support PNG files. Then, every time I work on the project locally or build the project with CI, it will also download a JPEG, SVG and potentially dozens of other supported minifiers, make those available despite the fact I don't want them and still make them available.

And like I said before, the fact that this is the first time I encountered this issue despite working on many projects with many dependencies, indicates to me that the majority of packages don't use peerDependencies this way.

However, I'm also not saying that the way this lib is using this option is wrong. I just don't know.

But then, I'm also not sure if simply moving those dependencies from peerDependencies to devDependencies would solve the issue. I'm sure this was one of the things I tried to do last weekend, but to be honest, I can't remember now, why I then continued to change things around further. 😵‍💫

[sebbo2002]

For example, popular packages like image minifiers that support optional third-party packages, still do not list them as peerDependencies (see imagemin or next-optimized-images).

Both of your examples are JavaScript examples, so they automatically don't have the problem: the third-party libraries are only needed for typing. Otherwise, no dependencies are included, so the JavaScript gets by completely without these third-party libraries.

But then, I'm also not sure if simply moving those dependencies from peerDependencies to devDependencies would solve the issue.

I don't think so, because if you look in the package.json, it's already there. In peerDependencies and in the devDependencies.

[Manc]

I think I remember now why moving the optional libraries to devDependencies would also not solve the issue: When I install the package (ical-generator) in my own project via a NPM dependency, more or less only the dist folder will be included in the node_modules directory, which means the TypeScript typings for third-party dependencies are being lost again, yet ical-generator would use them, which would then result in an error in my own project.

The devDependencies only relate to the dependencies for developing the actual package/library the package.json belongs to, not projects using them.

[Manc]

Out of interest, I just created a new minimal project using NPM version 7.12.1 with dependencies typescript and ical-generator. I added a small tsconfig.json and my main script is this:

import ical from 'ical-generator';
console.log(ical().toString());

Running tsc gave me the same errors that I get in my other project:

➜  calcal npm run build

> [email protected] build
> tsc

node_modules/ical-generator/dist/calendar.d.ts:1:23 - error TS2688: Cannot find type definition file for 'node'.

1 /// <reference types="node" />
                        ~~~~

node_modules/ical-generator/dist/calendar.d.ts:2:31 - error TS2307: Cannot find module 'moment-timezone' or its corresponding type declarations.

2 import type { Duration } from 'moment-timezone';
                                ~~~~~~~~~~~~~~~~~

node_modules/ical-generator/dist/calendar.d.ts:4:32 - error TS2307: Cannot find module 'http' or its corresponding type declarations.

4 import { ServerResponse } from 'http';
                                 ~~~~~~

node_modules/ical-generator/dist/calendar.d.ts:347:35 - error TS2503: Cannot find namespace 'NodeJS'.

347     save(path: string, cb?: (err: NodeJS.ErrnoException | null) => void): this;
                                      ~~~~~~

node_modules/ical-generator/dist/event.d.ts:1:28 - error TS2307: Cannot find module 'rrule' or its corresponding type declarations.

1 import type { RRule } from 'rrule';
                             ~~~~~~~

node_modules/ical-generator/dist/tools.d.ts:1:39 - error TS2307: Cannot find module 'moment' or its corresponding type declarations.

1 import type { Moment, Duration } from 'moment';
                                        ~~~~~~~~

node_modules/ical-generator/dist/tools.d.ts:2:41 - error TS2307: Cannot find module 'moment-timezone' or its corresponding type declarations.

2 import type { Moment as MomentTZ } from 'moment-timezone';
                                          ~~~~~~~~~~~~~~~~~

node_modules/ical-generator/dist/tools.d.ts:3:28 - error TS2307: Cannot find module 'dayjs' or its corresponding type declarations.

3 import type { Dayjs } from 'dayjs';
                             ~~~~~~~

node_modules/ical-generator/dist/tools.d.ts:4:48 - error TS2307: Cannot find module 'luxon' or its corresponding type declarations.

4 import type { DateTime as LuxonDateTime } from 'luxon';
                                                 ~~~~~~~

node_modules/ical-generator/dist/tools.d.ts:5:28 - error TS2307: Cannot find module 'rrule' or its corresponding type declarations.

5 import type { RRule } from 'rrule';
                             ~~~~~~~

node_modules/ical-generator/dist/types.d.ts:1:29 - error TS2307: Cannot find module 'moment' or its corresponding type declarations.

1 import type { Moment } from 'moment';
                              ~~~~~~~~

node_modules/ical-generator/dist/types.d.ts:2:41 - error TS2307: Cannot find module 'moment-timezone' or its corresponding type declarations.

2 import type { Moment as MomentTZ } from 'moment-timezone';
                                          ~~~~~~~~~~~~~~~~~

node_modules/ical-generator/dist/types.d.ts:3:28 - error TS2307: Cannot find module 'dayjs' or its corresponding type declarations.

3 import type { Dayjs } from 'dayjs';
                             ~~~~~~~

node_modules/ical-generator/dist/types.d.ts:4:48 - error TS2307: Cannot find module 'luxon' or its corresponding type declarations.

4 import type { DateTime as LuxonDateTime } from 'luxon';
                                                 ~~~~~~~


Found 14 errors.

It looks like to me that using NPM 7 does not solve the issue either.

[sebbo2002]

I actually tested this at the time, but can do it again tonight. If it doesn't work with npm 7 (anymore), I have to change something, of course.

[Manc]

If it helps, I've pushed my sample project: https://github.com/Manc/cal-example

[sebbo2002]

Okay, the modules are no longer installed automatically because of this commit, as they were marked as optional. That is, of course, unfavourable.

I understand that it can be desirable to have to install as few modules as possible, but the pure use of native Date is currently not an option, as time zones are only insufficiently supported here. And these are in turn necessary when it comes to repeating events, for example.

I will expand the FAQ in the README accordingly and ask Typescript users to install these modules manually. I think this is currently the most sensible way to solve this dilemma. JavaScript users do not need these dependencies and after the Typescript code is built, they are no longer needed, so they do not have to be included in productive containers or the like.