Modernize this library

[davidgoli]

The first commits in this library were from 2012, and since then, the JavaScript ecosystem has advanced considerably. Language features which were not available at the time are now commonplace, in particular, babel and webpack.

Additionally, this library's main source code is all production-ready in lib/rrule.js, a 2288-line file that is very difficult to understand and maintain, and also is constrained by a need to be browser-compatible.

In the existence of tools like Babel and Webpack, there is no need for JavaScript source code to be browser-executable, since these tools handle the transpilation to a distribution bundle very efficiently. In fact, very few modern libraries still handle their own browser compatibility.

I would like to propose a pull request that will:

• Introduce a build step using babel and webpack (and, if the other maintainers are interested, typescript).
• Break down lib/rrule.js into multiple files that can be isolated from each other and have proper unit tests written around each one of them, so that their role/responsibilities can be clearly articulated and understood by new contributors - candidates include dateutil, weekday, rrulestr, helpers, iterresult and others.
• Leave existing functionality untouched: Nothing added, nothing removed, nothing altered.
• Not introduce any new package.json depedencies outside of devDependencies
• Check in package-lock.json
• (Possibly) introduce new unit tests around each of these extracted files

This could open the door to a further breakdown of the remaining rrule.js, so that ideally no single file is more than a couple hundred lines. I do notice there appears to be what I think may be some concern in the codebase about the performance implications of function call overhead, leading to very long, very nested functions instead of more isolated unit functions - is that accurate?

I'm looking at packages like lodash for inspiration here, though lodash has a build system that is probably more complex than needed here for the time being.

Opening this issue for discussion. Thoughts? Ideas? Concerns?

[arolson101]

Sounds good to me. There’s not much in the way of active development on this project, so any and all contributions are welcome.

That being said, you’re in for an uphill battle. The code looks to be originally ported from python to js, and it’s pretty hard to follow. So long as the tests all still work I’m ok with it.

And excited! I work with typescript so I’m partial to that, but as long as we have typescript definitions I’m fine with whatever.

[davidgoli]

@arolson101 I've pushed up a first pass at this in PR #225 , I decided adopting Typescript was going to be enough to do in its own PR - this one is big enough already. Would love to hear any and all feedback!

[davidgoli]

Question for folks! While I'm checking in a lockfile, is there a preference for npm (via package-lock.json) or yarn (via yarn.lock)?

[jorroll]

+1 for yarn

[davidgoli]

Fixed with #225