From e6bda02eee82f74ac26cc502f16035cef0b676cf Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Thu, 22 Aug 2024 11:00:51 -0400 Subject: [PATCH 1/3] SWC-7038 - fix e2e tests --- e2e/files.spec.ts | 5 ++++- e2e/helpers/testUser.ts | 24 +++++++++--------------- e2e/homepage.spec.ts | 6 +++--- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/e2e/files.spec.ts b/e2e/files.spec.ts index a8762f0be4..db737567dc 100644 --- a/e2e/files.spec.ts +++ b/e2e/files.spec.ts @@ -356,7 +356,10 @@ testAuth.describe('Files', () => { await expect( userPage.getByRole('heading', { name: userProject.name }), ).toBeVisible() - await userPage.getByRole('link', { name: 'Files', exact: true }).click() + await userPage + .getByRole('link', { name: 'Files', exact: true }) + .first() + .click() }) await testAuth.step('upload file', async () => { diff --git a/e2e/helpers/testUser.ts b/e2e/helpers/testUser.ts index 32886c9d9e..6063521975 100644 --- a/e2e/helpers/testUser.ts +++ b/e2e/helpers/testUser.ts @@ -134,13 +134,11 @@ export async function loginTestUser( export async function goToDashboard(page: Page) { await page.goto('/') - await waitForInitialPageLoad(page) - const authenticatedUserBundleResponsePromise = getAuthenticatedUserBundleResponsePromise(page) - await page.getByRole('link', { name: 'View Your Dashboard' }).first().click() // wait for page to load + await waitForInitialPageLoad(page) await authenticatedUserBundleResponsePromise await expectAuthenticatedNavDrawerLoaded(page) } @@ -189,7 +187,7 @@ export async function getUserIdFromLocalStorage(page: Page) { } export async function acceptSiteCookies(page: Page) { - await page.getByRole('button', { name: 'Accept and continue' }).click() + await page.getByRole('button', { name: 'ALLOW ALL' }).click() } export const dismissAlert = async (page: Page, alertText: string) => { @@ -260,18 +258,14 @@ export const expectDiscussionThreadLoaded = async ( export const toggleIntoExperimentalMode = async (page: Page) => { await test.step('toggle into experimental mode', async () => { - await expect( - page.getByRole('link', { name: 'Experimental Mode Off' }), - ).toBeVisible() + const experimentalModeLabel = page.getByText('Experimental Mode') + const experimentalModeInput = page.getByLabel('Experimental Mode') - await page.getByRole('link', { name: 'Experimental Mode' }).click() - await expect( - page.getByRole('heading', { name: 'Experimental Mode' }), - ).toBeVisible() - await page.getByRole('button', { name: 'OK' }).click() + await expect(experimentalModeInput).not.toBeChecked() - await expect( - page.getByRole('link', { name: 'Experimental Mode On' }), - ).toBeVisible() + // Because the input element is not visible, we need to click the label + await experimentalModeLabel.click() + + await expect(experimentalModeInput).toBeChecked() }) } diff --git a/e2e/homepage.spec.ts b/e2e/homepage.spec.ts index e8a5892e9c..f5f347669a 100644 --- a/e2e/homepage.spec.ts +++ b/e2e/homepage.spec.ts @@ -11,7 +11,7 @@ test.describe('Homepage - Unauthenticated', () => { await expect( page.getByRole('link', { name: 'Log in to Synapse' }), - ).toHaveCount(2) + ).toHaveCount(1) await expect( page.getByRole('link', { name: 'View Your Dashboard' }), ).toHaveCount(0) @@ -22,7 +22,7 @@ testAuth.describe('Homepage - Authenticated', () => { testAuth( 'should show View Your Dashboard button when logged in', async ({ userPage }) => { - await userPage.goto('/') + await userPage.goto('/Home:x') await waitForInitialPageLoad(userPage) await expect( @@ -30,7 +30,7 @@ testAuth.describe('Homepage - Authenticated', () => { ).toHaveCount(0) await expect( userPage.getByRole('link', { name: 'View Your Dashboard' }), - ).toHaveCount(2) + ).toHaveCount(1) }, ) }) From 4e83b1903b3779d80bebaf13ac05c1b08b271dea Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Wed, 28 Aug 2024 12:48:45 -0400 Subject: [PATCH 2/3] SWC-7038 - Fix issue where React would try to unmount a ReactDOM root before the current render task finished. See https://github.com/facebook/react/issues/25675 --- .../web/client/widget/ReactComponent.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java index 610c028032..592448665a 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java @@ -7,6 +7,7 @@ import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.event.dom.client.HasClickHandlers; import com.google.gwt.event.shared.HandlerRegistration; +import com.google.gwt.user.client.Timer; import com.google.gwt.user.client.ui.ComplexPanel; import com.google.gwt.user.client.ui.Widget; import java.util.ArrayList; @@ -175,9 +176,17 @@ protected void onLoad() { protected void onUnload() { super.onUnload(); if (root != null) { - root.unmount(); + // Asynchronously schedule unmounting the root to allow React to finish the current render cycle. + // https://github.com/facebook/react/issues/25675 + Timer t = new Timer() { + @Override + public void run() { + root.unmount(); + root = null; + } + }; + t.schedule(0); } - root = null; } /** From 122660b09e6f420f78a9e47c9c5530b256424a27 Mon Sep 17 00:00:00 2001 From: Nick Grosenbacher Date: Thu, 29 Aug 2024 17:05:13 -0400 Subject: [PATCH 3/3] SWC-7038 - React component render should also be scheduled --- .../web/client/widget/ReactComponent.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java index 592448665a..88fdf77487 100644 --- a/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java +++ b/src/main/java/org/sagebionetworks/web/client/widget/ReactComponent.java @@ -143,15 +143,22 @@ private void injectChildWidgetsIntoComponent() { public void render(ReactNode reactNode) { this.reactElement = reactNode; - maybeCreateRoot(); injectChildWidgetsIntoComponent(); // This component may be a React child of another component. If so, we must rerender the ancestor component(s) so // that they use the new ReactElement created in this render step. ReactComponent componentToRender = getRootReactComponentWidget(); if (componentToRender == this) { - // Resynchronize with the DOM - root.render(this.reactElement); + // Asynchronously schedule creating a root in case React is still rendering and may unmount the current root + Timer t = new Timer() { + @Override + public void run() { + maybeCreateRoot(); + // Resynchronize with the DOM + root.render(reactElement); + } + }; + t.schedule(0); } else { // Walk up the tree to the parent and repeat rerendering componentToRender.rerender();