search

jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.3k stars 513 forks source link

Luxonless binary should not contain any luxon imports. #410

Closed samouri closed 4 years ago

samouri commented 4 years ago

summary Fixes https://github.com/jakubroztocil/rrule/issues/344, and maybe https://github.com/jakubroztocil/rrule/issues/402.

Currently the build produced without timezone support still references luxon and expects the code to throw a TypeError if incorrectly used (done via webpack externals). You can verify this by searching for "luxon" within the contents of dist/es5/rrule.min.js.

This causes issue when mixed with compilers that don't have the same notion of an external, e.g. Closure Compiler (https://github.com/google/closure-compiler/issues/954). Specifically, the AMP Project is running into when trying to build rrule into our amp-date-picker component (https://github.com/ampproject/amphtml/pull/28887).

This PR uses NormalModuleReplacementPlugin to replace luxon with a fake implementation that immediately throws.

samouri commented 4 years ago

@jakubroztocil / @davidgoli: friendly ping 🏓

jkbrzt commented 4 years ago

Thanks!

samouri commented 4 years ago

Thank you for merging this fix! Not that there's any rush, but what's the release process like?

jkbrzt commented 4 years ago

@samouri I hope to find a time for it over the weekend (unless @davidgoli beats me to it).

Out of curiosity—what is your rrule.js use case?

samouri commented 4 years ago

@samouri I hope to find a time for it over the weekend (unless @davidgoli beats me to it).

Awesome!

Out of curiosity—what is your rrule.js use case?

I work for AMP, and we use rrule.js in the amp-date-picker component to support the highlighted/blocked attributes. We've managed to work around this issue in the meantime by manually patching the node_modules folder. See:

https://github.com/ampproject/amphtml/blob/a28fa129d225ce79d6b537f500c4540ae844caae/build-system/tasks/update-packages.js#L108-L119

jkbrzt commented 4 years ago

@samouri thanks for the background. ~v2.6.5 is out — https://www.npmjs.com/package/rrule~ The build process is broken, will need to fix that first.