Update front end dependencies

[jasmine-q]

I went ahead and ran yarn upgrade-interactive --latest and checked everything.
Ran the app and tests, then fixed any breaking changes

Notable changes

• updated to jest 28
• added jest-environment-jsdom
• added resolutions for jsdom dependency tough-cookie to 4.0.0 due to breaking changes with localhost domains

☝️ for this last point we should probably change it to use localhost.local or something later on?

Note: all the snapshots tests fail on my local, is that intentional?

[kalopilato]

Looks great @jasmine-q! I ran up and the app and smoke tested - looks like everything's working as expected (ah chirps... brings back memories 😀)

Ran the tests and checked in on my interview repo - the date/locale related snapshot failures were there for me too so I think that's intentional?

[jasmine-q]

Looks great @jasmine-q! I ran up and the app and smoke tested - looks like everything's working as expected (ah chirps... brings back memories 😀)

Ran the tests and checked in on my interview repo - the date/locale related snapshot failures were there for me too so I think that's intentional?

Thanks for the review @kalopilato!!
I'll see if @ticky has any additional context/thoughts before I merge 👀

[ticky]

Ermmm I don't think it's intentional, can you shoot me the error you're getting?

[kalopilato]

Ermmm I don't think it's intentional, can you shoot me the error you're getting?

@ticky There are 9 test snapshot failures, all on RelativeDateTime.spec.js along these lines:

I noted them in my interview too, and at the time just assumed they were part of the assessment 🤷 (I didn't fix them though, I chose to put my time elsewhere)

[ticky]

Ohhh, hmm, yeah I think that's actually an oversight. I guess we're not specifying the locale, so it's whatever the local user is set to.

[jasmine-q]

Ohhh, hmm, yeah I think that's actually an oversight. I guess we're not specifying the locale, so it's whatever the local user is set to.

@ticky do we want to fix this or leave it as part of the "coding challenge"?

[ticky]

Yeah, maybe we just lock to a known locale here:

kitesocial-js/app/javascript/components/RelativeDateTime.js

Line 27 in 47659c8

title={date.toLocaleString(DateTime.DATETIME_SHORT)}

should be as simple as changing it to:

date.toLocaleString(DateTime.DATETIME_SHORT, { locale: 'en-AU' })

[AeroCross]

As a quick exercise, I tried applying the update suggested by @ticky but it didn't seem to work.

Snapshot Summary
 › 15 snapshots failed from 3 test suites. Inspect your code changes or re-run jest with `-u` to update them.

Test Suites: 3 failed, 1 passed, 4 total
Tests:       13 failed, 2 passed, 15 total
Snapshots:   15 failed, 3 passed, 18 total
Time:        0.935 s

I didn't really want to dedicate too much time into this, so I can't be for absolute certain if my setup (node 18.8.0 w/ jest 29.0.2) is the root cause of this issue.

I am wondering if there's anyone that feels that fixing these specs as part of the exercise could be beneficial or detrimental for a candidate's performance? Happy to go with my gut feel, but getting the braintrust's input is always a 👌 experience in my eyes.

[jasmine-q]

As a quick exercise, I tried applying the update suggested by @ticky but it didn't seem to work.

Snapshot Summary
 › 15 snapshots failed from 3 test suites. Inspect your code changes or re-run jest with `-u` to update them.

Test Suites: 3 failed, 1 passed, 4 total
Tests:       13 failed, 2 passed, 15 total
Snapshots:   15 failed, 3 passed, 18 total
Time:        0.935 s

I didn't really want to dedicate too much time into this, so I can't be for absolute certain if my setup (node 18.8.0 w/ jest 29.0.2) is the root cause of this issue.

I am wondering if there's anyone that feels that fixing these specs as part of the exercise could be beneficial or detrimental for a candidate's performance? Happy to go with my gut feel, but getting the braintrust's input is always a 👌 experience in my eyes.

Thanks for trying to set the locale @AeroCross! Please try pulling it down and running the tests now.
I ended up setting the locale in RelativeDateTime and also setting the timezone in the jest global setup to ensure snapshots remain up to date.
I tried lining up the timezone and the locale to the original snapshot timezone (Canada/Pacific) but no joy. Even if the time matched up the format was now dd/mm/yyyy AM/PM instead of the original yyyy-mm-dd a.m/p.m

[kalopilato]

Nice one @jasmine-q 👌 That worked a treat!

I reckon the date format change is inconsequential in this case, and updating the snapshots was a good call - not worth extra braining to match the pre-existing snapshots 👍