Crashes without luxon

[froatsnook]

Luxon is an optional dependency and I'd rather not include it since my project already has moment and I don't need timezone support in my rrule usage.

So I installed it:

$ npm install --save --no-optional rrule
+ rrule 2.6.0
$  ls node_modules | grep luxon
# nothing

So far so good. But then as soon as I bundle with webpack and run, it crashes in ./node_modules/rrule/dist/esm/src/datewithzone.js with "Module not found: Error: Can't resolve 'luxon'". Is this a limitation of webpack? How can I use rrule without luxon?

[davidgoli]

Hi, are you able to create a sample project that reproduces this? Thanks!

[jorroll]

So the issue here appears to be that, currently, luxon is actually a required dependency (not optional). This package's main entry point, src/index.ts, depends on src/rrule.ts which depends on src/iter/index.ts which depends on src/datewithzone.ts which depends on luxon (all es6 static imports are required to be resolvable). The fact that this issue doesn't come up more frequently just goes to show how few people are interested in using this library without timezone support.

In order to make luxon an optional dependency, it either needs to be dynamically imported (via import('luxon').then()) and hidden behind a conditional, or the paths which require luxon need to be moved into a separate (and optional) package entry point.

The dynamic import route might look like this:

// src/datewithzone.ts
import dateutil from './dateutil'

try {
  import('luxon').then(luxon => DateWithZone.luxon = luxon).catch()
} catch (e) {}

export class DateWithZone {
  static luxon: { fromJSDate(date: Date): any } | undefined;

  // other stuff ...

  public rezonedDate () {
    if (this.isUTC) {
      return this.date
    }
    else if (!DateWithZone.luxon) {
      console.error(
        'Using TZID without Luxon available is unsupported. ' +
        'Returned times are in UTC, not the requested time zone'
      )

      return this.date;
    }

    const datetime = DateWithZone.luxon
      .fromJSDate(this.date)

    const rezoned = datetime.setZone(this.tzid!, { keepLocalTime: true })

    return rezoned.toJSDate()
  }
}

There are a few problems with this though.

1. Theoretically, this opens up the possibility of a race condition if DateWithZone is used before `import('luxon') has loaded.
2. I'm not sure how bundlers would treat the dynamic import. They might use code splitting to move it into a separate bundle which would then need to get loaded separately. The end user's web app would need to be able to dynamically load this code. Anyway, it makes things more complicated.

I think moving this code into a separate and optional package entry point is the right solution. This might look like:

// src/datewithzone.ts
import dateutil from './dateutil'

export class DateWithZone {
  static luxon: { fromJSDate(date: Date): any } | undefined;

  // other stuff ...

  public rezonedDate () {
    if (this.isUTC) {
      return this.date
    }
    else if (!DateWithZone.luxon) {
      console.error(
        'Using TZID without Luxon available is unsupported. ' +
        'Returned times are in UTC, not the requested time zone'
      )

      return this.date;
    }

    const datetime = DateWithZone.luxon
        .fromJSDate(this.date)

    const rezoned = datetime.setZone(this.tzid!, { keepLocalTime: true })

    return rezoned.toJSDate()
  }
}

and

// src/luxon/index.ts
import { DateWithZone } from '../datewithzone';
import { DateTime } from 'luxon';

DateWithZone.luxon = DateTime;

Usage would be like

import { RRule } from 'rrule';
import 'rrule/luxon';

// ... do stuff

A separate, sideEffects: true package.json would need to be added to src/luxon/package.json to allow usage of the new entry point.

This would also be a breaking change, as all apps making use of rrule's timezone support (which appears to be 99% of folks) would need to import 'rrule/luxon';. For an app depending on time zone support, they'll want to make sure that all code paths making use of rrule have also imported rrule/luxon. Rather than adding import 'rrule/luxon' to each file making use of rrule, they can add a local ./rrule.ts file that looks like so:

// ./rrule.ts
import 'rrule/luxon';
export * from 'rrule';

And then import from this local file, rather than rrule directly. They could also simply add import 'rrule/luxon' in their app's bootstrapping file, so long as that file is run before any other rrule code.

Edit:
Obviously this solution complicates the usage process (and again, is a breaking change). Given how infrequently this issue has come up, a fair response might be to simply make luxon a required dependency of rrule. It appears that 99% of users are already happy using rrule with luxon side by side--even if they don't realize that they are doing so.

[AntonSermyazhko]

Hi,
What is the progress on this issue?

[jorroll]

All the progress is contained on this page.

[KrakenTyio]

After few months i try to remove from project luxon but still see this issue

Error: ./node_modules/rrule/dist/esm/src/datewithzone.js
Module not found: Error: Can't resolve 'luxon' in '/home/node/app/node_modules/rrule/dist/esm/src'

project is with angular 11 and we already using moment.js

"rrule": "~2.6.8",

[OldShaterhan]

Similar issue happened for me. In my case downgrade from npm 7 to npm 6 resolves the issue, but it isn't target solution.