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

Update to Node 18.15 #8631

Merged
merged 12 commits into from Mar 10, 2023
2 changes: 1 addition & 1 deletion .github/actions/build-frontend/action.yml
Expand Up @@ -28,7 +28,7 @@ runs:
- name: Use Node.js ${{ matrix.node-version }} with yarn
uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c
with:
node-version: "16.19.1"
node-version: "18.14.2"

Choose a reason for hiding this comment

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

I wonder if we can just use node-version-file so we can just maintain the version in .nvmrc instead of having to keep this updated as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@JosiahSiegel feasible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If devops approves, I feel this would be the right way to go.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, according to the docs (see: https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#node-version-file) it can read the engines.node property in package.json so the .nvmrc file may not be needed.

Choose a reason for hiding this comment

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

Oh, fantastic! Anything automatic like this would be the way to go in my opinion 🙏


- run: yarn install
working-directory: frontend-react
Expand Down
2 changes: 1 addition & 1 deletion frontend-react/.nvmrc
@@ -1 +1 @@
16.19.1
18.14.2
2 changes: 1 addition & 1 deletion frontend-react/package.json
Expand Up @@ -190,6 +190,6 @@
"workbox-webpack-plugin": "^6.5.4"
},
"engines": {
"node": "~16.19.1"
"node": "~18.14.2"
}
}
Expand Up @@ -3,8 +3,10 @@ import userEvent from "@testing-library/user-event";

import { ReceiverData } from "../../config/endpoints/messageTracker";
import { renderApp } from "../../utils/CustomRenderUtils";
import { createProxyNodeDateTimeFormatter } from "../../utils/TestUtils";

import {
dateTimeFormatter,
FilterOption,
formatMessages,
MessageReceivers,
Expand Down Expand Up @@ -98,8 +100,13 @@ const sortScenarios: {
];

describe("formatMessages function", () => {
// Proxy formatter so we can replace node's unicode spaces
const alteredFormatter =
createProxyNodeDateTimeFormatter(dateTimeFormatter);
test("formats data properly", () => {
expect(formatMessages(MOCK_RECEIVER_DATA)).toEqual(formattedMockData);
expect(formatMessages(MOCK_RECEIVER_DATA, alteredFormatter)).toEqual(
formattedMockData
);
});
});

Expand Down
Expand Up @@ -266,7 +266,10 @@ export const MessageReceiversRow = ({
// fileName: string;
// transportResults: string;
// }
export function formatMessages(data: ReceiverData[]): NormalizedReceiverData[] {
export function formatMessages(
jpandersen87 marked this conversation as resolved.
Show resolved Hide resolved
data: ReceiverData[],
formatter: Intl.DateTimeFormat
): NormalizedReceiverData[] {
return data.map((receiverItem: ReceiverData): NormalizedReceiverData => {
const formattedData: NormalizedReceiverData = Object.keys(
ColumnDataTitles
Expand All @@ -289,7 +292,7 @@ export function formatMessages(data: ReceiverData[]): NormalizedReceiverData[] {
break;
case ColumnDataTitles.date:
if (receiverItem.createdAt)
formattedData.date = dateTimeFormatter.format(
formattedData.date = formatter.format(
new Date(receiverItem.createdAt)
);
break;
Expand Down Expand Up @@ -364,7 +367,7 @@ export const MessageReceivers = ({ receiverDetails }: MessageReceiverProps) => {
const [activeColumnSortOrder, setActiveColumnSortOrder] =
useState<FilterOption>(FilterOptionsEnum.NONE);
const normalizedData = useMemo(
() => formatMessages(receiverDetails),
() => formatMessages(receiverDetails, dateTimeFormatter),
[receiverDetails]
);
const sortedData = useMemo(
Expand Down
40 changes: 40 additions & 0 deletions frontend-react/src/utils/TestUtils.ts
Expand Up @@ -48,3 +48,43 @@ export function conditionallySuppressConsole(...matchers: string[]) {
jestWarn.mockRestore();
};
}

/**
* Future updates to browser Intl will eventually include a change to strings where
* some spaces will be unicode no-break spaces (u+202F / charCode 8239) instead of
* regular spaces (charCode 32). This apparently already happened in the browser
* space but was quickly reverted due to breaking WebCompat. Node 18.14.2
* implements this change which will break tests doing string comparisons of Intl
* output with developer-crafted comparisons. A future update to Node may revert
* this change as well if this was reverted in V8 itself. In either case, this
* helper will be important for the eventual transition period if they persue this
* again. Use this helper function preferably within a mock implementation or
* proxy object handler so the strings are passed on naturally.
*/
export function replaceUnicodeNarrowNoBreakSpaces(str: string) {
return str.replaceAll(String.fromCharCode(8239), String.fromCharCode(32));
}

/**
* Create a proxy of a Node DateTimeFormat object so we can replace unicode no-break
* strings before returning.
*/
export function createProxyNodeDateTimeFormatter(
formatter: Intl.DateTimeFormat
) {
return new Proxy(formatter, {
get(target, p) {
const targets: (string | symbol)[] = [
"format",
"formatRange",
] as (keyof Intl.DateTimeFormat)[];

if (targets.includes(p)) {
return (...args: any[]) =>
replaceUnicodeNarrowNoBreakSpaces(target.format(...args));
}

return (target as any)[p];
},
});
}