Partial for #3252: new Date() -> Date.now()
[x] Updated Changelog [x] Partial fix for #3252 (documentation is in a separate repo)
Btw, this probably shouldn't be in the PR template as 100+ unrelated files changed:
- Please run
npm run lint:prettierbefore submitting so that style issues are fixed.
I can remove it from the template .md if you want. Or do a separate PR with all the style issues fixed :P
Also, would be nice if we had a way to enforce people not using new Date() in new code, maybe a change to pull-request rules, or an automated test that scans the code? I'm open to ideas.
would be nice if we had a way to enforce people not using new Date() in new code
The prefer-date-now rule of the unicorn ESLint plugin seems to be a good solution for that.
but teaching others the right way is a long haul problem.
everybody has to quit breaking old code.
would be nice if we had a way to enforce people not using new Date() in new code
The prefer-date-now rule of the unicorn ESLint plugin seems to be a good solution for that.
oh that's cool, this would be ideal. I'm not sure how to hook it up though.
@KristjanESPERANTO @sdetweil I could use some help with this. In calendar.js, this works:
createEventList (limitNumberOfEntries) {
const ONE_SECOND = 1000; // 1,000 milliseconds
const ONE_MINUTE = ONE_SECOND * 60;
const ONE_HOUR = ONE_MINUTE * 60;
const ONE_DAY = ONE_HOUR * 24;
const now = new Date(); // new Date(Date.now()); // breaks tests
const today = moment().startOf("day");
const future = moment().startOf("day").add(this.config.maximumNumberOfDays, "days").toDate();
let events = [];
But when I change this part:
const now = new Date(Date.now()); // breaks tests
const today = moment(Date.now()).startOf("day");
const future = moment(Date.now()).startOf("day").add(this.config.maximumNumberOfDays, "days").toDate();
then some of the electron tests like has css class dayBeforeYesterday hang and time out after 20 sec. It's pretty easy to repro, but I'm not sure how to debug the electron tests.
I'm not sure if the current construction is o.k. but I can explain what happens.
If you change the one line then Date.now() is already mocked in calendar.js which is not the case with the old line.
The mocking happens in tests/electron/helpers/global-setup.js.
The tests time out because the expected css property never appears.
I don't have a better suggestion yet, but somehow new Date(Date.now()) seems a bit too hacky to me. Is there perhaps a more elegant approach?