Refactor to work with Tree Shaking

[SLaks]

Motivation

I wrote some very simple code to run in a browser and print a full year of פרשה names:

import { HebrewCalendar } from '@hebcal/core'
const events = HebrewCalendar.calendar({
  year: new Date().getFullYear() - 1,
  numYears: 2,
  sedrot: true,
  noHolidays: true,
})

console.log(
  events.map((event) => ({
    datetime: event.date.greg(),
    label: event.render('he-x-NoNikud'),
  }))
)

When compiled for production using https://vitejs.dev/ with default settings, this produced 150 kB of optimized JS!

dist/index-Cca6IFY6.js                    150.13 kB │ gzip: 53.69 kB

Since all functions are contained in the HebrewCalendar class and the Event hierarchy, Tree Shaking cannot see which APIs are actually used.

Suggestion

It ought to be possible to build a new modular API in which application code only imports features that are actually used. This should allow tree shaking to entirely drop unused features (eg, emojis, zmanim calculations, unused locales, etc).

This would be a new API that requires explicit imports to enable more calculations and methods. The existing API would remain as a wrapper to avoid breaking existing code. Applications that want to benefit from tree shaking would need to migrate to the new API to gain the benefits.

Details

Making code tree-shakable involves a number of changes (these changes can be made independently for incremental improvements, but each incremental change would require clients to refactor further to get more benefit):

Note: All of these suggestions are rough ideas; I have not reviewed the code carefully enough to be sure that each of these make sense and would actually allow non-trivial amounts of code to be tree-shaken

• Replace the static methods on HebrewCalendar with separate exports to drop unused methods.
  • This is the simplest improvement, and should be able to drop files like tachanun.ts and hallel.ts easily.
• Replace locale name strings with separately-exported locale objects containing their translations.
  • This way, if a project only uses one locale, other locales will be dropped.
  • Unless you expect projects to use both he and he-x-nonikud, it would also make sense to move he-x-nonikud to a separate file generated at build time, so that projects that only use it can drop nikud entirely.
• Replace the flags enum with an array of importable objects that contain their Event subclasses and the relevant parts of getHolidaysForYear_()
  • This way, projects that only use certain flags can drop all code for other events.
  • Fully implementing this would involve a major refactor of getHolidaysForYear_(); we should try to estimate how much code this could actually save first.
• Similarly to the previous bullet, replace the various boolean options in HebrewCalendar.calendar() with an array of imported functions or plugins to specify which events you want returned.
  • This would let projects drop all of the Zmanim code (if they don't pass candleLighting or omer), which should be a very big win.
  • This can also improve end-user type safety; you can declare the function as returning an array of Event subclasses based on the plugins you pass.
  • This can be implemented in stages; declare all of the plugins first (so that consumers can import only what they need), and you can then slowly split apart the underlying code to be more modular. This allows you to make incremental improvements under the hood without forcing application code to change further.
  • Similarly, you could eventually split apart getHolidaysForYear_() based on plugins as well for further savings.
• Move any expensive members of Event and subclasses (eg, getEmoji(); I don't know if there are any other costly members) to a top-level exported function that takes the event as a parameter instead.
  • To be less disruptive, you could declare an exported function that adds getEmoji() to Event.prototype, so that it is still available as a member function, but only if this is imported.
  • This can even maintain full type safety in TypeScript; example (TypeScript will only see this file, and therefore the prototype declaration, if imported).
  • I haven't tried it, but you can probably make this even safer in TypeScript by declaring the importable installer function as asserts Event is {prototype: {getEmoji(): string}}

Let me know if you need help understanding, designing, prototyping, or implementing this.

[mjradwin]

If it's really important to reduce bundle size right now before undertaking such an ambitious refactor, we could do a minor refactor and move Sedra class (and related parshiyot array) from @hebcal/core to @hebcal/hdate and your app could be changed to depend only on the latter. Or make a @hebcal/sedra package that depends only on the latter. Either way it could be done in a way that doesn't break compatibility for @hebcal/core clients because we would still roll it up within@hebcal/core

[SLaks]

I use HebrewCalendar.calendar() here, so that won't help. On further thought, though, I could probably drop that and instead loop over all dates and simply filter out those that have no leinings.

If I do that, I could probably fix this much more simply by making getSedra_() public.

Either way (even if do the full split/refactor), I would also need to refactor hebcal/hebcal-leyning to remove all references to HebrewCalendar; see hebcal/hebcal-leyning#467.
In particular, it calls HebrewCalendar.getHolidaysOnDate(). The simplest approach there is to move it to holidays.ts without changing the API at all (and make the original wrap the moved version). I don't know how much code holidays.ts itself depends on, but it doesn't seem to have any large, non-core dependencies (HolidayEvent and staticHolidays are pretty large, but those are always required), so it probably isn't worth doing any further refactors (dropping YomKippurKatanEvent and modern.ts wouldn't save much code)

Tree shaking should then automatically remove the large dependencies (the entire HebrewCalendar class and everything it references), since I would no longer reference it at all.

I don't think this is particularly urgent for me right now, but I did start working on it.

[mjradwin]

Looping over all dates is pretty easy. You'll find this idiom in a few places:

const startAbs = HDate.hebrew2abs(year, TISHREI, 1);
const endAbs = HDate.hebrew2abs(year + 1, TISHREI, 1) - 1;
for (let absDt = startAbs; absDt <= endAbs; absDt++) {
  const events = HebrewCalendar.getHolidaysOnDate(absDt);
  if (events) {
    // ...
  }
}

[mjradwin]

Taking a look at this, I have a question for you about how to avoid pulling the Zmanim dependency without breaking backwards compatibility

• holidays.ts depends on HolidayEvent, which is used for most of your typical holidays
• HolidayEvent has optional properties startEvent and endEvent which are both of type TimedEvent. For a calendar that includes no zmanim, these will always be undefined. However, if the user specifies a location for candle-lighting times, the event instance for a fast day (for example Tzom Gedaliah) will have those two properties as references to a "Fast begins" event and a "Fast ends" event. You'll see where these properties get set in https://github.com/hebcal/hebcal-es6/blob/main/src/candles.ts#L76
• TimedEvent depends on Location and Zmanim, which pulls in the huge dependencies you are trying to avoid

If we wanted to remove that dependency, how would we handle it? As I'm relatively new to TS, I'm totally open to your suggestions! Do we remove startEvent and endEvent properties from HolidayEvent, and create a FastDayEvent subclass that has those properties?

[SLaks]

Done in https://github.com/hebcal/hebcal-es6/commit/c31e0076f0f3669258e0cca0f78f391ab0e078f6

Sorry; I already implemented all of this (except refactoring the calendar() API; I'm going to follow your advice and use a loop instead).

I'm now testing it against leyning.

[mjradwin]

Oh, sorry, I didn't realize you were working on this today. I apologize that my getHolidaysOnDate refactor probably caused a bit of a merge mess for you

[SLaks]

Your refactor is actually a duplicate of https://github.com/hebcal/hebcal-es6/commit/d0be1bb04c73922018685b42b4b9195451b3cf08, except that I didn't touch getHolidaysForYearArray

[SLaks]

Current Status

From my POV, this is complete. With https://github.com/hebcal/hebcal-es6/pull/481 and https://github.com/hebcal/hebcal-leyning/pull/473, https://github.com/SLaks/tikkun.io/tree/use-modular works.

The refactor shrinks the Tikkun's index chunk from 258KB to 151KB!

Possible next steps (I probably will not do these):

• Refactor calendar() as described above (I stopped using this function entirely, as you recommended; it's probably faster without).
  • If you want to do this, you should probably either delete or rename the current calendar.ts, so that the new modular version can be imported from a nice path.
• I just noticed that @hebcal/hdate uses (internally only) dist/esm, whereas I used dist/es; you may want to rename either one to be consistent.
• https://github.com/hebcal/hebcal-es6/commit/048f73f0708af283951940a7d81a394c6db2ea58 will probably cause issues for downstream projects that import @hebcal/core and try to use @hebcal/leyning, since those two imports will use different .d.ts files (so the types won't match). I did not actually try this. Since I dropped ts-imp, I should no longer need that.

[mjradwin]

Thanks for this update. So glad this is working.

I think I can safely revert https://github.com/hebcal/hebcal-es6/commit/048f73f0708af283951940a7d81a394c6db2ea58 as you're the one who originally asked for a way to append suffixes in https://github.com/hebcal/hebcal-leyning/issues/432 and if that's no longer an issue, we may as well go back to the simpler build process and avoid duplicate .d.ts