Frontend dependency updates #5721

tsmock · 2023-04-19T18:25:07Z

This PR intends to make the TM compatible with the defaults of Debian Bookworm. This means that the frontend needs to be compatible with NodeJS 18.x.

In general, the code changes are intended to be limited to compatibility changes. In the event that there needs to be a reformat due to formatting tool updates, those will be in separate commits (to make rebasing onto a new develop HEAD easier).

This PR replaces frontend PRs #5712, #5640, #5586, #5585, #5583, #5540, #5529, #5510, #5474, #5467, and #5177.

This does not update Rapid to v2 -- that will need to be a separate PR.

Total bundle size has decreased from 14.26 MB to 8.07 MB.
main bundle size has decreased from 3.12 MB to 2.56 MB.

This can be additionally reduced by lazy-loading the map component

At one point, I had the main bundle size reduced to 700 KB (before I updated the react-scripts to 5.0.1, which required a workaround for facebook/create-react-app#11770 ). Either that update or the workaround caused two bundles to merge; the first with our code, the second with most of the "heavy" modules.

Test runs from #5721

File	Test	RPi (Node 16, develop)	Mac (Node 16, updates)	Mac (Node 18, updates)
src/components/projectCard/tests/dueDateBox.test.js	test DueDate › with tooltip message			1
src/components/projectDetail/tests/questionsAndComments.test.js	test if QuestionsAndComments component › renders tabs for writing and previewing comments		1	1
src/components/projectDetail/tests/shareButton.test.js	test if shareButton › render open corresponding window popup			1
src/components/projectDetail/tests/shareButton.test.js	test if shareButton › render shareButton for project with id 1			1
src/components/taskSelection/tests/actionSidebars.test.js	Appearance of unsaved map changes to be dealt with while mapping › when splitting a task		2	1
src/components/taskSelection/tests/actionSidebars.test.js	Appearance of unsaved map changes to be dealt with while mapping › when submitting a task			1
src/components/taskSelection/tests/actionSidebars.test.js	Completion Tab for Validation › should update status and comments for multiple tasks		1
src/components/taskSelection/tests/contributions.test.js	Contributions › clean user selection if we click on the selected tasks of the user	1
src/components/taskSelection/tests/contributions.test.js	Contributions › render users, links, and	1	1	1
src/components/taskSelection/tests/index.test.js	Contributions › should clear text when close icon is clicked	3
src/components/taskSelection/tests/index.test.js	Contributions › should select tasks mapped by the selected user	10
src/components/taskSelection/tests/index.test.js	Contributions › should select tasks validated by the selected user	10
src/components/taskSelection/tests/index.test.js	Contributions › should sort tasks by their task number	10
src/components/taskSelection/tests/taskActivity.test.js	TaskHistory › renders the task history comments and activities for a given project		1	1
src/components/taskSelection/tests/taskList.test.js	Task Item › should display task detail modal		1	1
src/components/tests/selectAll.test.js	SelectAll › unactive when clicked select all elements			1
src/views/tests/interests.test.js	Edit Interest › should hide the save button on click			1
src/views/tests/interests.test.js	Edit Interest › should return input text value to default when cancel button is clicked			1
src/views/tests/management.test.js	Management Page Overview Section › should list projects and teams fetched	2
src/views/tests/project.test.js	CreateProject renders ProjectCreate		1
src/views/tests/project.test.js	Project Detail Page › should render component details	10	1
src/views/tests/projectStats.test.js	ProjectStats dashboard › fetch urls and render sections title		1	2
src/views/tests/taskSelection.test.js	Random Task Selection › should not change the button text to map selected task when user selects a task for mapping	10
src/views/tests/taskSelection.test.js	Task Selection Page › should change the button text to map another task when user selects a task for validation but the user level is not met	7
src/views/tests/taskSelection.test.js	Task Selection Page › should change the button text to map selected task when user selects a task	10	1	1
src/views/tests/taskSelection.test.js	Task Selection Page › should change the button text to validate selected task	10	1	1
src/views/tests/taskSelection.test.js	Task Selection Page › should filter the task list by search query	8	2	1
src/views/tests/teams.test.js	Edit Team › should display default details of the team before editing			2
src/views/tests/userDetail.test.js	User Detail Component › should render detail for the child components		1	1

[1] bundleSizes.zip

HelNershingThapa

I have reviewed most of the changes overall and find them satisfactory. I have provided my comments where I felt there was room for improvement or further attention. Great job on addressing the annoying warnings across the app! I can't help but add a little levity here:
"Knock, knock."
"Who's there?"
"Warnings."
"Warnings who?"
"No warnings because Taylor has squashed them all!"

It's fantastic to see most of those warnings disappear. There are currently some failing test cases in the codebase that requires revision. I'd like to reassess the modifications when it's rebased to develop. Thanks.

EDIT:
The image in the testimonial section on the landing page is not displayed.

frontend/package.json

frontend/src/utils/polyfill.js

frontend/src/components/footer/styles.scss

frontend/src/components/projectDetail/styles.scss

HelNershingThapa · 2023-05-15T11:09:45Z

frontend/src/components/projectDetail/timeline.js

-        scales: { xAxes: [{ type: 'time', time: { unit: unit } }] },
+        scales: {
+          y: { ticks: { beginAtZero: true } },
+          x: { type: 'time', time: { unit: unit }, adapters: { date: { locale: enUS } } },


There seems to be a noticeable difference in the appearance of the chart and units before and after the change.

Before:

After:

Furthermore, it appears that there is a new addition to the chart tooltip, displaying the time. Previously, the time was not included in the tooltip.

I didn't consider this -- I was testing against a local DB, so I had very few data points. The problem here is that the latter is more accurate when showing when contributions occurred relative to other data points. I don't know who implemented the original timeline, but we might want to loop them in for this conversation. I can see it going either way. There is a timeseries type we can use if the original plot was the intended behavior.

I don't have knowledge of the original timeline implementation. @ramyaragupathy, I'm looping you into this conversation. You might have more insights or context that can help us make an informed decision.

@HelNershingThapa @tsmock - monthly view would work only for projects that are running for a longer time, for ones that have been active for only a week or so would the x axis automatically be using date instead of month? Ideally it will be good to have a toggle between month and date view for the chart. For now we can stick to dates instead of months for the x-axis.

I've pulled the fixes for the console warnings out into #5964.

frontend/package.json

tsmock · 2023-05-15T13:37:12Z

Great job on addressing the annoying warnings across the app! I can't help but add a little levity here: "Knock, knock." "Who's there?" "Warnings." "Warnings who?" "No warnings because Taylor has squashed them all!"

I haven't hit all the warnings. Some would require significant changes (for example, there is a warning on the project list about <a> inside <a>, IIRC).

It's fantastic to see most of those warnings disappear. There are currently some failing test cases in the codebase that requires revision. I'd like to reassess the modifications when it's rebased to develop. Thanks.

Yep. I had them all passing before my last rebase, and was going to work through them this week.

EDIT: The image in the testimonial section on the landing page is not displayed.

Thanks for noticing. I'll double check and see what happened. I don't recall removing any images/image code.

HelNershingThapa · 2023-05-15T14:24:09Z

Thanks for noticing. I'll double check and see what happened. I don't recall removing any images/image code.

I think I ran into the same problem when upgrading to React 18. I was able to resolve it with b7ff508.

HelNershingThapa · 2023-05-16T07:58:09Z

The test execution using fetch generates numerous warnings that indicate a captured request without a corresponding request handler in the MSW library. Here's the specific warning message:

console.warn
    [MSW] Warning: captured a request without a matching request handler:
    
      • GET http://localhost/undefined
    
    If you still wish to intercept this unhandled request, please create a request handler for it.
    Read more: https://mswjs.io/docs/getting-started/mocks

      at Object.warn (node_modules/msw/src/utils/internal/devUtils.ts:17:11)
      at applyStrategy (node_modules/msw/src/utils/request/onUnhandledRequest.ts:208:18)
      at onUnhandledRequest (node_modules/msw/src/utils/request/onUnhandledRequest.ts:233:3)
      at handleRequest (node_modules/msw/src/utils/handleRequest.ts:80:5)
      at node_modules/msw/src/node/SetupServerApi.ts:69:24

No warning was encountered when testing a component that utilizes Axios for data fetching. It seems likely that the warnings are associated with MSW's specific handling mechanisms for the native fetch API in Node.js 18.

EDIT: MSW will ship with Node.js 18+ support with MSW v2.0 as mentioned here mswjs/msw#1388 (comment).

tsmock · 2023-05-24T15:32:00Z

For the MSW issue, I did a bit of debugging.

    [MSW] Warning: captured a request without a matching request handler:
    
      • GET http://localhost/undefined

It turns out this is a problem from the @mswjs/interceptors package (we are currently using 0.17.9). The code used expects the input to either be a string or have a url field (see v0.17.8/src/interceptors/fetch/index.ts). Our current code calls fetch with URL objects, which are not strings and do not have a url field.

This is fixed in later versions of @mswjs/interceptors (starting in v0.18.0). We would need to update msw to their next release.

The remaining tests that are failing are due to timeout issues, and are not very consistent. Should I increase the timeouts for them?

HelNershingThapa · 2023-05-24T16:05:32Z

I find it puzzling that this seemingly simple test takes longer than the 5-second timeout. Because the test appears to be a straightforward simple one, as it seeks to verify the successful opening of a dialog with specific text and expects the dialog to close upon clicking the close button.

Despite that, I'm open to the notion of increasing the timeouts for these specific tests. I've also noticed some of these test cases failing in my local development setup but passing in the CircleCI ones. This discrepancy between the environments adds another layer of complexity to the situation.

tsmock · 2023-05-24T17:29:02Z

It would be a lot easier if it was consistent. I've had tests fail and then pass, only to fail again later.

I think it is probably related to a greater usage of React.lazy and React.suspense (specifically related to splitting translations out of the main bundle).

HelNershingThapa · 2023-05-24T17:59:37Z

If that were the case, shouldn't a lot of these test cases fail?

tsmock · 2023-05-24T18:08:50Z

That would be intuitive. However, that is the only thing that I saw in common when I was spot checking the failing tests. I'll drop the timeouts I added in some commits and see if I can otherwise fix the issue (maybe I can make a mock that will fix the issue).

EDIT: Nevermind. Both of the tests that failed when I dropped some timeout changes had CommentInputField in common as well.

HelNershingThapa · 2023-05-25T15:57:26Z

I noticed that the circle layer for the tasks centroid on the project detail page does not navigate to the task selection page when clicked. This functionality appears to be broken.

https://github.com/facebook/OSM-HOT-Tasking-Manager/blob/1ff61218946e43cec7f66c65dc225ac8410b7fa8/frontend/src/components/taskSelection/map.js#L399-L400

EDIT: Something unusual is happening here: when you press the back button on the browser numerous times, you'll find that the app did navigate to the intended route, indicating that the on-click event is being triggered multiple times. We can eliminate the problem by removing the relative navigation and instead using the full path '/projects/:id/tasks,' but the back functionality still won't work.

tsmock · 2023-05-25T18:55:15Z

I checked on the tasks-stage instance, and I had to click back 10 times to go back a page.

As far as going to the wrong page, here is the calls that leads to the incorrect behavior:

/projects/:id + ./tasks -> /projects/:id/tasks
/projects/:id/tasks +./tasks -> /projects/:id/tasks/tasks
/projects/:id/tasks/tasks + ./tasks -> /tasks
/tasks + ./tasks -> /tasks

The current state of code probably triggers the call navigate calls another 6 times.

EDIT: I've made a PR for this at #5962.

tsmock · 2023-05-31T12:54:39Z

I think I'm going to start splitting some of the changes out. Specifically,

a93d478 (Improve treeshaking)
dd5d203 (Fix some forwardRef warnings)
00a5c6b (ReactJS: Don't use 'selected' for '<option/>' nodes, ReactJS: Don't use 'selected' for '<option/>' nodes #5965)
~~cb1e391 (ReactJS: Fix an input component changing from uncontrolled to controlled, ReactJS: Fix an input component changing from uncontrolled to controlled #5890)~~
bd155aa (React: Fix "Invalid DOM property class" in JSX SVG, React: Fix "Invalid DOM property class" in JSX SVG #5963)
b7ee5c1 (ReactJS: Fix "<div> cannot appear as a descendant of <p>" warnings)
~~59bbde8 (React: Fix "Each child in a list should have a unique 'key' prop", ReactJS: Fix "Each child in a list should have a unique 'key' prop" #5891)~~
~~ebfc712 (ReactJS: Fix an input component changing from uncontrolled to controlled, ReactJS: Fix an input component changing from uncontrolled to controlled #5890)~~
~~1b41079 (Tests: add missing await for waitFor statements, Tests: add missing await for async statements in tests #5871)~~
~~7415db6 (Fix eslint warnings for valid-typeof, Fix eslint warnings for valid-typeof #5889)~~
~~56a54cc (Use userEvent.setup and move existing cases to recommended location, Frontend Tests: Standardize userEvent usage to conform with upstream recomendations #5872)~~
a0e5402 (Wrap methods which affect UI by act in tests)
c473fd5 (Drop polyfills)

Signed-off-by: Taylor Smock <[email protected]>

quickstart.js has the same spacing as it had previously (which comes from the empty <p/>). Signed-off-by: Taylor Smock <[email protected]>

* `Sign Up` button * comment boxes * actionsForm.js (mostly buttons) Signed-off-by: Taylor Smock <[email protected]>

This reduces the main chunk size from 3.65 MB to 2.56 MB. Something about the commit "Use CRACO to workaround CRA #11770" caused several large components to be added to the main.*.js bundle. All suspense elements are *roughly* the same size as the expected text editor box, plus or minus a row. Signed-off-by: Taylor Smock <[email protected]>

* Avoid default imports in ReactJS There were quite a few files where we imported react, but didn't actually use it. * Use non-default imports where possible * Use named imports where possible Signed-off-by: Taylor Smock <[email protected]>

Signed-off-by: Taylor Smock <[email protected]>

… handling Signed-off-by: Taylor Smock <[email protected]>

Signed-off-by: Taylor Smock <[email protected]>

…g to finish Signed-off-by: Taylor Smock <[email protected]>

Signed-off-by: Taylor Smock <[email protected]>

This should fix some timeout issues.

Signed-off-by: Taylor Smock <[email protected]>

sonarcloud · 2024-04-16T14:18:29Z

Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

varun2948 · 2024-04-29T05:12:13Z

@ramyaragupathy I think we are good to go on this PR and we are ready to merge it into staging for now and will create another PR fixing the osm download most probably today and will let manjita test it.
Cc: @manjitapandey, @royallsilwallz

github-actions bot added scope: frontend scope: infrastructure dependencies Pull requests that update a dependency file javascript scope: translations labels Apr 19, 2023

tsmock force-pushed the chore/frontend-dependency-cleanup branch 2 times, most recently from 3398ff9 to 7450a7a Compare April 20, 2023 20:44

tsmock marked this pull request as ready for review April 20, 2023 20:45

ramyaragupathy mentioned this pull request May 9, 2023

[BUG] NodeJS v16 used for frontend is EOL in September 2023 #5767

Closed

eternaltyro requested a review from HelNershingThapa May 9, 2023 09:50

HelNershingThapa reviewed May 15, 2023

View reviewed changes

tsmock force-pushed the chore/frontend-dependency-cleanup branch from 7edbab9 to 3af2c03 Compare May 15, 2023 17:56

tsmock force-pushed the chore/frontend-dependency-cleanup branch from 7f91467 to 5377f20 Compare May 17, 2023 21:05

eternaltyro self-assigned this May 23, 2023

tsmock force-pushed the chore/frontend-dependency-cleanup branch 2 times, most recently from 91ecdad to dbec7f5 Compare May 24, 2023 15:09

tsmock force-pushed the chore/frontend-dependency-cleanup branch from dbec7f5 to 1ff6121 Compare May 25, 2023 14:36

tsmock force-pushed the chore/frontend-dependency-cleanup branch from 13def81 to e9c741a Compare May 31, 2023 12:47

tsmock force-pushed the chore/frontend-dependency-cleanup branch from e9c741a to b90a6dc Compare June 2, 2023 12:51

tsmock and others added 17 commits April 16, 2024 08:16

Fix tests

e1ce970

Signed-off-by: Taylor Smock <[email protected]>

ReactJS: Fix "<div> cannot appear as a descendant of <p>" warnings

190f75e

quickstart.js has the same spacing as it had previously (which comes from the empty <p/>). Signed-off-by: Taylor Smock <[email protected]>

Fix forwardRef warnings

f4cf85d

* `Sign Up` button * comment boxes * actionsForm.js (mostly buttons) Signed-off-by: Taylor Smock <[email protected]>

Improve treeshaking

c14841b

* Avoid default imports in ReactJS There were quite a few files where we imported react, but didn't actually use it. * Use non-default imports where possible * Use named imports where possible Signed-off-by: Taylor Smock <[email protected]>

fixup! Improve treeshaking

a5724da

Signed-off-by: Taylor Smock <[email protected]>

Revise implementation to insert testimonial image

da410aa

Fix tests again

978c2ad

Signed-off-by: Taylor Smock <[email protected]>

internationalization: Fix Sonar issues for promises with no exception…

191c6f6

… handling Signed-off-by: Taylor Smock <[email protected]>

Add JOSM responses to server-handlers

845d617

Signed-off-by: Taylor Smock <[email protected]>

Fix navigation on point click

bf33971

Signed-off-by: Taylor Smock <[email protected]>

Testing-Library: Increase default timeout

de0b494

Signed-off-by: Taylor Smock <[email protected]>

project.test.js: Attempt to fix resource leak by waiting for renderin…

e53fe0b

…g to finish Signed-off-by: Taylor Smock <[email protected]>

Fix Rapid imports for newer build-time dependencies

2a911ca

Signed-off-by: Taylor Smock <[email protected]>

Cancel call to ohsome when a component is unloaded

56da86b

Signed-off-by: Taylor Smock <[email protected]>

Explicitly import lazy imports in tests

e8e93dc

This should fix some timeout issues.

Fix failing test in projectStats.test.js by waiting for text

67ad269

Signed-off-by: Taylor Smock <[email protected]>

tsmock force-pushed the chore/frontend-dependency-cleanup branch from 826ed49 to 67ad269 Compare April 16, 2024 14:17

ramyaragupathy mentioned this pull request Apr 17, 2024

Update query-string to v8.1.0 from v7.1.3 #5892

Closed

manjitapandey mentioned this pull request Apr 18, 2024

[BUG] OSM download components crashes due to node version upgrade to 18 #6345

Closed

ramyaragupathy added this to the v4.7.2 milestone Apr 23, 2024

ramyaragupathy merged commit 3115315 into hotosm:develop Apr 30, 2024
6 of 8 checks passed

This was referenced May 1, 2024

[BUG] frontend build fails #5659

Closed

Add a temporary workaround to get building on Node 18 since Node 16 EOL is 2023-09-11 #6053

Closed

tsmock mentioned this pull request May 1, 2024

Upgrade to React 18 #5510

Closed

This was referenced May 2, 2024

feat: fixed osm download destroy issue #6375

Merged

[BUG] CORS error in Dev server #6404

Closed

royallsilwallz mentioned this pull request Jun 12, 2024

Page goes blank when user selects language to Netherlands #6425

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frontend dependency updates #5721

Frontend dependency updates #5721

tsmock commented Apr 19, 2023 •

edited

Loading

HelNershingThapa left a comment •

edited

Loading

HelNershingThapa May 15, 2023

tsmock May 15, 2023

HelNershingThapa May 15, 2023

ramyaragupathy Jun 6, 2023

tsmock Jul 6, 2023

tsmock commented May 15, 2023

HelNershingThapa commented May 15, 2023

HelNershingThapa commented May 16, 2023 •

edited

Loading

tsmock commented May 24, 2023

HelNershingThapa commented May 24, 2023

tsmock commented May 24, 2023

HelNershingThapa commented May 24, 2023

tsmock commented May 24, 2023 •

edited

Loading

HelNershingThapa commented May 25, 2023 •

edited

Loading

tsmock commented May 25, 2023 •

edited

Loading

tsmock commented May 31, 2023 •

edited

Loading

sonarcloud bot commented Apr 16, 2024

varun2948 commented Apr 29, 2024

Frontend dependency updates #5721

Frontend dependency updates #5721

Conversation

tsmock commented Apr 19, 2023 • edited Loading

HelNershingThapa left a comment • edited Loading

Choose a reason for hiding this comment

HelNershingThapa May 15, 2023

Choose a reason for hiding this comment

tsmock May 15, 2023

Choose a reason for hiding this comment

HelNershingThapa May 15, 2023

Choose a reason for hiding this comment

ramyaragupathy Jun 6, 2023

Choose a reason for hiding this comment

tsmock Jul 6, 2023

Choose a reason for hiding this comment

tsmock commented May 15, 2023

HelNershingThapa commented May 15, 2023

HelNershingThapa commented May 16, 2023 • edited Loading

tsmock commented May 24, 2023

HelNershingThapa commented May 24, 2023

tsmock commented May 24, 2023

HelNershingThapa commented May 24, 2023

tsmock commented May 24, 2023 • edited Loading

HelNershingThapa commented May 25, 2023 • edited Loading

tsmock commented May 25, 2023 • edited Loading

tsmock commented May 31, 2023 • edited Loading

sonarcloud bot commented Apr 16, 2024

Quality Gate passed

varun2948 commented Apr 29, 2024

tsmock commented Apr 19, 2023 •

edited

Loading

HelNershingThapa left a comment •

edited

Loading

HelNershingThapa commented May 16, 2023 •

edited

Loading

tsmock commented May 24, 2023 •

edited

Loading

HelNershingThapa commented May 25, 2023 •

edited

Loading

tsmock commented May 25, 2023 •

edited

Loading

tsmock commented May 31, 2023 •

edited

Loading