Deprecate Recurr in favor of php-rrule

[codyogden]

Hi Marc,

I hope you're doing well. :)

I'd like to propose changing the underlying RRule library from Recurr to php-rrule.

As I was digging into my current project where I'm utilizing the ACF RRule Plugin, I've noticed some issues with Recurr as a library. Easily noted is that the Recurr library is not updated frequently and had gone nearly a year without any updates. It has months-old pending PRs to bring it into RFC compliance (specifically a big bug around parsing RRules with timezones/TZID listed).

The php-rrule project seems to have a much more reliable beat/tempo of commits by the maintainer(s), issues and PRs don't seem nearly as stale. I've played around with it in a scratchpad PHP env, and it seems pretty solid. I also like that it has a build in method occursAt() which enables the ability to check a date against an existing RRule. It also offers features around finding occurrences between two points in time, so we could implement some sort of pagination functionality like I discussed in #3.

I'm going to go ahead and implement replacing Recurr with php-rrule in my custom version of ACF RRule Plugin that I'm using, but I'm wondering how you'd feel if I PRed this replacement into the project?

[marcbelletre]

Hi Cody,

I'm doing great thank you! How about you?

I didn't run into any issues with Recurr for now so thank you for pointing out the lack of support with this library. There are indeed some years old issues like #137 that were never solved although a new release was pushed a few weeks ago.

The php-rrule library on the other hand looks promising and is actively maintained. Also if it can help us implementing the pagination feature we were talking about then I am for sure not opposed to switching to it as long as we can assure backwards compatibility with the current version.

I look forward for your PR!

Thanks again for your support :)

[bhujagendra-ishaya]

Hi Marc

I have created a patch to include php-rule as suggested in this post. However, at closer look, it does not necessarily seem to be a wise choice.

The main issue is that the RRule object is "immutable". I have partly solved this by creating a deriving class, that allows values to be set after instantiation of the object (as it is used in "update_value"). However, this is not complete, as currently the __toString()-function relies on the values that are stored in the $rule-property array, rather than the properties that are set by the setters at the moment. So the setters would have to set the property in the rule as well.

Then there are limitations with php-rule as well - or maybe with the implementation of this field in general: while there is sort of support for DTSTART, there is no such support of EXDATE or RDATE which both influence the produced series of dates.

So what I am looking for is a way to implement more into the ACF field. I was wondering if there is interest from your side on this as well.

If yes, I would create a new issue for the discussion of that. If no, I'd probably start a fork and we can always merge together later.

Thank you for your work and time!
Bhjuagenda

[marcbelletre]

Hi Bhujagendra,

Thanks again for your work on this.

I don't think we need support for EXDATE or RDATE. There is no support in simshaun/recurr as well.
There are other ways to manage exceptions like creating a separate repeater field with date fields so you can exclude them later in a custom function. Or you can use the RRule field inside of a repeater field so you can create multiple periods.
That being said it would be great if we could manage them directly inside the RRule field.

I'm personally happy with the current implementation. Switching to php-rrule was meant to solve the RFC compliance issue and I don't think we absolutely need that as long as we are able to convert a single string to a series of dates in the end.

For now as far as I can see there is no advantage to switching to php-rrule as it does not solve the RFC compliance issue (we would have to take off the DTSTART attribute from the string) and it seems to bring a bit more complexity to the current implementation.

Of course I would be happy to take your contributions if you want to bring new features to the plugin. Feel free to open a new discussion ;)

Thank you!