-
-
Notifications
You must be signed in to change notification settings - Fork 737
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
chore: fix traffic data timezone visualization issue with getUTCDate #9110
chore: fix traffic data timezone visualization issue with getUTCDate #9110
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
@@ -132,7 +132,7 @@ const toChartData = ( | |||
|
|||
for (const dayKey in item.days) { | |||
const day = item.days[dayKey]; | |||
const dayNum = new Date(Date.parse(day.day)).getDate(); | |||
const dayNum = new Date(Date.parse(day.day)).getUTCDate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a test for the function generating those dates that fails with previous code and succeeds with the new one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
He showed me the test but it needs to run vite with a different timezone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, yep fully onboard with that @kwasniew - but I'm yet to find a good way to have a committable repro test for this :( Looking into spoofing timezones and such, but doesn't bite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may not be worth the effort. ideally before the test we should change the timezone and reset to original value afterward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another option would be to avoid overriding the timezone in the tested method explicitly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sth like this. haven't checked if it works
const originalTimeZone = process.env.TZ;
beforeEach(() => {
// Set the timezone to the desired value
process.env.TZ = 'America/New_York';
});
afterEach(() => {
// Reset the timezone to the original value
process.env.TZ = originalTimeZone;
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying that but can't get it to "bite" unfortunately
Dates are hard. For a lot of timezones new Date(Date.parse('2025-01-01T00:00:00.000Z')).getDate() could be returned as 31 and not 1
By using getUTCDate instead we now adjust accordingly and return the right day number.