DEP Upgrade front-end build stack #1389

GuySartorelli · 2022-11-03T00:51:50Z

Parent issue

Upgrade frontend build stack for CMS5 #1312

GuySartorelli · 2022-12-08T03:29:59Z

client/src/components/ActionMenu/ActionMenu.js

-    this.setState({
-      isOpen: !this.state.isOpen
-    });
+    // Force setting state to the end of the execution queue to clear a potential race condition
+    // with entwine click handlers
+    window.setTimeout(() => this.setState({ isOpen: !this.state.isOpen }), 0);


I've added this in a couple of places defensively - the only place I know for certain it's needed is this one.
The race condition occurs like this:

User clicks the button

React responds first.

React toggles the menu to close, which removes the button that was clicked

Entwine wants to respond to the event, but the button isn't in the DOM so it doesn't.

This solution works because it defers closing the dropdown (and thus removing the button) until the next event cycle (see the MDN docs for setTimeout). This allows the current event cycle to continue (and therefore lets entwine respond to the click event) before closing closing the dropdown.
More about the event loop can be read in MDN's docs on the subject.

GuySartorelli · 2022-12-12T03:29:46Z

client/src/components/DatetimeField/tests/DatetimeField-test.js

-      expect(inputField.value).toBe('2017-01-05T02:23');
+      // Note that the inclusion of seconds or milliseconds is unnecessary but acceptable
+      expect(inputField.value).toMatch(/^2017-01-05T02:23(:22|:22.000)?$/);


The test was failing because the value included the seconds and milliseconds. The actual functionality was fine, as this is still a valid ISO format.

GuySartorelli · 2022-12-12T03:30:37Z

client/src/components/GridFieldActions/GridFieldActions.js

-  constructor(props) {
-    super(props);
-
-    this.toggle = this.toggle.bind(this);
-    this.state = {
-      dropdownOpen: false
-    };
-  }
-
-  toggle() {
-    this.setState({
-      dropdownOpen: !this.state.dropdownOpen
-    });
-  }


The toggle method and dropdownOpen state were never being referenced anywhere so I've removed them.

GuySartorelli · 2022-12-12T03:34:59Z

client/src/components/Link/Link.js

+/**
+ * This component can be used whenever a component which needs a relative link
+ * might be rendered in both context within a router (e.g. asset admin) and
+ * contexts without a router (e.g. inside an image upload modal).
+ */
+function Link({ children, href, ...props }) {


This is necessary because the update of react-router resulted in the way routes are added being changed - where the route and url both used to be e.g. /admin/assets/show/1/edit, the route is now relative to the base route, e.g. assets/show/1/edit. This means we have to use a react Link component when in a react router context so that the correct full path is on the rendered <a> tag's href attribute..

GuySartorelli · 2022-12-12T03:35:47Z

client/src/components/ResizeAware/ResizeAware.js

@@ -8,7 +8,7 @@ import PropTypes from 'prop-types';
 *
 * Adapted from https://github.com/FezVrasta/react-resize-aware created by Federico Zivolo.
 */
-export default class ResizeAware extends Component {


Move export to after the proptypes and defaultprops are declared.

GuySartorelli · 2022-12-12T03:37:45Z

client/src/components/TreeDropdownField/TreeDropdownField.js

+
+  return <CacheProvider value={cache}>{children}</CacheProvider>;
+}
+
 class TreeDropdownField extends Component {


Most of the changes in this class are a result of changes in the API for react-select.
See the upgrade guides here: https://react-select.com/upgrade we went from v1 all the way to v5.

GuySartorelli · 2022-12-12T03:38:49Z

client/src/components/TreeDropdownField/TreeDropdownField.js

+   * Render the breadcrumbs.
+   * This sits above the options in nested trees, as a way to navigate back
+   */
+  renderBreadcrumbs(breadcrumbs, { cx, getStyles, getClassNames, ...props }) {


This used to be in TreeDropdownFieldMenu which is no longer needed.

GuySartorelli · 2022-12-12T03:41:18Z

client/src/components/TreeDropdownField/TreeDropdownField.js

+      : title;
+  }
+
+  noOptionsMessage({ inputValue }) {


This used to be in TreeDropdownFieldMenu which is no longer needed.

GuySartorelli · 2022-12-12T03:41:44Z

client/src/components/TreeDropdownField/TreeDropdownField.js

    }

-    const resetValue = (this.props.data.hasEmptyDefault && !this.props.visible.length)


No longer needed

GuySartorelli · 2022-12-12T03:45:13Z

client/src/containers/App/App.js

-  <div className="app">{renderRoutes(route.routes())}</div>
+  <div className="app"><Outlet /></div>


Rendering the routes is no longer the responsibility of this component. It's handled by BootRoutes.js explicitly.

GuySartorelli · 2022-12-12T03:47:53Z

client/src/legacy/AddToCampaignForm.js

-      ReactDOM.render(
+      let root = this.getReactRoot();
+      if (!root) {
+        root = createRoot(this[0]);
+      }
+      root.render(


ReactDOM.render was deprecated in react 17 and removed in react 18. It would still technically work but it would result in react behaving like it was still v17. See this blog post for more details.
See also _clearModal() below for the replacement of the old ReactDOM.unmountComponentAtNode() method.

GuySartorelli · 2022-12-12T03:51:27Z

client/src/lib/dependency-injection/tests/ReducerContainer-test.js

-jest.mock('../MiddlewareRegistry', () => () => ({ add: jest.fn() }));
+jest.mock('../MiddlewareRegistry', () => jest.fn().mockImplementation(() => ({ add: () => {} })));


This was necessary to fix an error where the mocked class wasn't being returned correctly. The error was along the lines of MiddlewareRegistry is not a constructor

GuySartorelli · 2022-12-13T01:36:16Z

JS CI build failed due to broken tests and linting errors. Those will be handled separately, see #1421 and #1411

GuySartorelli mentioned this pull request Nov 3, 2022

Upgrade frontend build stack for CMS5 #1312

Closed

7 tasks

GuySartorelli force-pushed the pulls/2/update-webpack branch 2 times, most recently from 78917e7 to 7d90c21 Compare November 15, 2022 03:42

GuySartorelli mentioned this pull request Nov 15, 2022

Wepbpack upgrade #1374

Closed

GuySartorelli force-pushed the pulls/2/update-webpack branch from 7d90c21 to b7d01d0 Compare November 22, 2022 01:21

GuySartorelli force-pushed the pulls/2/update-webpack branch 4 times, most recently from 4d2ee22 to b75a11c Compare December 2, 2022 03:08

GuySartorelli force-pushed the pulls/2/update-webpack branch 7 times, most recently from 3551c29 to 139e179 Compare December 8, 2022 01:46

GuySartorelli commented Dec 8, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/2/update-webpack branch 2 times, most recently from 1631d6c to 8c2bebf Compare December 12, 2022 03:12

GuySartorelli commented Dec 12, 2022

View reviewed changes

GuySartorelli mentioned this pull request Dec 13, 2022

CMS 5: Fix broken JavaScript tests #1421

Closed

2 tasks

emteknetnz approved these changes Dec 18, 2022

View reviewed changes

emteknetnz merged commit 2b8f53e into silverstripe:2 Dec 18, 2022

emteknetnz deleted the pulls/2/update-webpack branch December 18, 2022 21:19

GuySartorelli mentioned this pull request Feb 9, 2023

FIX Get JS tests passing again silverstripe/silverstripe-asset-admin#1330

Merged

GuySartorelli mentioned this pull request Feb 20, 2023

Some old outdated javascript dependencies need updating #1420

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEP Upgrade front-end build stack #1389

DEP Upgrade front-end build stack #1389

GuySartorelli commented Nov 3, 2022

GuySartorelli Dec 8, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli Dec 12, 2022

GuySartorelli commented Dec 13, 2022

		}

		const resetValue = (this.props.data.hasEmptyDefault && !this.props.visible.length)

		<div className="app">{renderRoutes(route.routes())}</div>
		<div className="app"><Outlet /></div>

		jest.mock('../MiddlewareRegistry', () => () => ({ add: jest.fn() }));
		jest.mock('../MiddlewareRegistry', () => jest.fn().mockImplementation(() => ({ add: () => {} })));

DEP Upgrade front-end build stack #1389

DEP Upgrade front-end build stack #1389

Conversation

GuySartorelli commented Nov 3, 2022

Parent issue

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuySartorelli commented Dec 13, 2022