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

dayOfWeek: [7] is not parsed with fieldsToExpression().stringify() #352

Open
arwa9 opened this issue Jun 5, 2024 · 2 comments
Open

dayOfWeek: [7] is not parsed with fieldsToExpression().stringify() #352

arwa9 opened this issue Jun 5, 2024 · 2 comments

Comments

@arwa9
Copy link

arwa9 commented Jun 5, 2024

When adding Sunday as value 7 inside daysOfWeek of the CronFields, and later parsing it to a cron expression with fieldsToExpression().stringify(), it is ignored.

Following the "Manipulation" example in the documentation:

var interval = parser.parseExpression('0 7 * * 0-4');
var fields = JSON.parse(JSON.stringify(interval.fields)); // Fields is immutable
fields.hour = [8];
fields.minute = [29];
fields.dayOfWeek = [1,3,4,5,6,7];
var modifiedInterval = parser.fieldsToExpression(fields);
var cronString = modifiedInterval.stringify();
console.log(cronString); // "29 8 * * 1,3-7"

When running said code, the actual output of that console.log() is "29 8 * * 1,3-6".

Seeing this should be handled here but doesn't appear to be working: https://github.com/harrisiirak/cron-parser/blob/master/lib/expression.js#L343

Also possible missing definition of sunday as 7 here (?): https://github.com/harrisiirak/cron-parser/blob/master/lib/expression.js#L109

@harrisiirak
Copy link
Owner

Hi @arwa9!

When parsing an expression, as fallback it supports day of week as range from 0-7. However, by the spec, day of week is defined as SUN - SAT, which translates to numeric range 0-6 (where 0 is SUN).

When serializing/stringifying interval, there is explicit behaviour to ignore 7 as day of week value.

If you want retain Sunday as part of serialized expression, Sunday must be presented as 0 and not 7.

Best regards

@arwa9
Copy link
Author

arwa9 commented Jun 5, 2024

Thank you for your quick response.

That's what I ended up doing, but then using a cron expression "translator" to human-readable expressions (like cronstrue ), this had the implication that those sentences become like "sunday and thursday to saturday" instead of "thursday to sunday".

Seeing the linux crontab reference I see the expected indexes for weekdays are indeed 0 to 6, but I became confused by the same exact code of your documentation I pasted in the issue, which I ran in an empty project and the output is not the expected as I mentioned.

If then using Sunday as 7 inside dayOfWeek for parsing purposes is purposefully being ignored, maybe it would be nice to not have that as an example in the documentation since it might lead to misunderstandings such as mine.

Thank you again for your time and keep up the good work!

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

No branches or pull requests

2 participants