-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor to work with Tree Shaking #433
Comments
Great suggestion! Unfortunately, I don't have the time to undertake such a large project -- I just started a new job and it may be many months before I could try to tackle this. I'm totally supportive of this kind of work if it can be done without breaking compatibility. Is this something you want to attempt? There was an earlier attempt to reduce the size of generated code with some modest success. As I was learning TypeScript, I split out To preserve backwards compatibility and to avoid breaking clients of |
I'm also busy with other stuff right now; I will probably take a stab at this in a month or two. I would probably do expose this as multiple files in a dedicated subfolder, so that application code would look like |
Moved to hebcal/hebcal-es6#475, because I filed this in the wrong repo |
Motivation
I wrote some very simple code to run in a browser and print a full year of פרשה names:
When compiled for production using https://vitejs.dev/ with default settings, this produced 150 kB of optimized JS!
Since all functions are contained in the
HebrewCalendar
class and theEvent
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
import
s 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
import
s 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
HebrewCalendar
with separateexport
s to drop unused methods.tachanun.ts
andhallel.ts
easily.he
andhe-x-nonikud
, it would also make sense to movehe-x-nonikud
to a separate file generated at build time, so that projects that only use it can drop nikud entirely.flags
enum with an array ofimport
able objects that contain theirEvent
subclasses and the relevant parts ofgetHolidaysForYear_()
getHolidaysForYear_()
; we should try to estimate how much code this could actually save first.HebrewCalendar.calendar()
with an array ofimport
ed functions or plugins to specify which events you want returned.candleLighting
oromer
), which should be a very big win.Event
subclasses based on the plugins you pass.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.getHolidaysForYear_()
based on plugins as well for further savings.Event
and subclasses (eg,getEmoji()
; I don't know if there are any other costly members) to a top-levelexport
ed function that takes theevent
as a parameter instead.export
ed function that addsgetEmoji()
toEvent.prototype
, so that it is still available as a member function, but only if this isimport
ed.prototype
declaration, if imported).import
able installer function asasserts Event is {prototype: {getEmoji(): string}}
Let me know if you need help understanding, designing, prototyping, or implementing this.
The text was updated successfully, but these errors were encountered: