Skip to content
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

Implement iterable/valueOf for decoupling from libraries #203

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andytson
Copy link
Contributor

@andytson andytson commented Jan 1, 2021

A dependent library could use this knowing only that expression: Iterable<{ valueOf(): number }> (where multiple date libraries support valueOf() already (including JS Date and luxon DateTime).

Fixes #82

A dependent library could use this knowing only that `expression: Iterable<{ valueOf(): number }>` (where multiple date libraries support valueOf() already (including JS Date and luxon DateTime).
@andytson
Copy link
Contributor Author

andytson commented Jan 1, 2021

An example I'm thinking of is a fork of node-scheduler that just implements one-off or iterable dates, so it doesn't need to bundle any specific cron-parser (and therefore date library), instead the user chooses this cron-parser and uses it like:

import { CronExpression } from 'cron-parser'
import { scheduleJob } from 'scheduler-fork'

scheduleJob(CronExpression.parse('*/20 * * * *'), () => ...)

i.e. I definitely want to avoid the moment-timezone bundle size/EOL but although my preference for change is Luxon, if instead DayJs is used, I can maintain a Luxon fork to avoid needing to bundle two date libraries in my project (and avoid DayJs timezone data).

@harrisiirak harrisiirak self-requested a review January 1, 2021 16:12
@andytson
Copy link
Contributor Author

andytson commented Jan 1, 2021

tbh, not having this isn't really a blocker to my idea. I'd just need an adapter to map the expression to the format my hypothetical node-scheduler fork would support without making it a hard dependency. The valueOf functionality would still be useful though as is standard in JS Date/some other date libraries

@andytson andytson mentioned this pull request Jan 10, 2021
@NGPixel
Copy link

NGPixel commented Oct 3, 2022

@harrisiirak Any update on merging this PR? Not being able to use for...of with the iterator flag is a bit annoying right now.


var iterated = [...interval].map((date) => date.toJSON());
var expected = [
"2012-12-26T14:40:00.000Z",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer single quotes over double quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep fair enough. I'm just used to eslint formatting on save :)

@harrisiirak
Copy link
Owner

Hi @NGPixel!

Thanks for reminding me about this! @andytson are you able to sync this PR branch with master branch to resolve conflict in the typings file? If not, I can create separate PR from your branch and merge it from there.

@andytson
Copy link
Contributor Author

andytson commented Oct 18, 2022

master has gone quite in a bit of a different direction since, at least with the CronExpression type, so may be tricky to sort out

@andytson
Copy link
Contributor Author

i.e I don't know how to integrate:

implements Iterable<CronDate> 

with

export type CronExpression<IsIterable extends boolean = false> = ICronExpression<CronFields, IsIterable>

so if you knew and could sort it out, that's fine.

@harrisiirak
Copy link
Owner

@andytson can't we just do:

export interface ICronExpression<CronFields, IsIterable extends boolean> extends Iterable<CronDate> {

Tested it locally and it seems to be working fine in this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iterate with for...of
3 participants