Migrate to MUI 5

[AresEkb]

The pull request: #3438

I suggest updating MUI to version 5 as Material UI 4 is outdated and does not receive security or performance updates. Also it's a complicated task to combine different versions of MUI in a single application. We managed it in our MUI 5 application based on Sirius Web, but it is really painful.

Also it will allow one to migrate to React 18 later.

What is changed

1. The following dependencies are updated:

-"@material-ui/core": "4.12.4",
-"@material-ui/icons": "4.11.3",
-"@material-ui/lab": "4.0.0-alpha.61",
+"@mui/material": "5.15.15",
+"@mui/icons-material": "5.15.15",
+"@mui/x-tree-view": "7.3.1",

2. Changed namespaces:

@material-ui/core -> @mui/material
@material-ui/icons -> @mui/icons-material

3. <TreeView> component moved to a separate project:

-import TreeView from '@material-ui/lab/TreeView';
-import { TreeItem } from '@material-ui/lab';
+import { SimpleTreeView } from '@mui/x-tree-view/SimpleTreeView';
+import { TreeItem } from '@mui/x-tree-view/TreeItem';

4. Changed <Autocomplete> component:

-import Autocomplete from '@material-ui/lab/Autocomplete';
+import Autocomplete from '@mui/material/Autocomplete';

It requires getOptionKey if labels are not unique

renderOption should return <li {...props}>

Nested <Chip> doesn't require key property, because it's already returned by getTagProps()

5. <TextField> and <Select> are outlined by default in MUI 5, so added the following to the theme:

{
  components: {
    MuiTextField: {
      defaultProps: {
        variant: 'standard',
      },
    },
    MuiSelect: {
      defaultProps: {
        variant: 'standard',
      },
    },
  },
},

6. makeStyles is replaced by TSS version

https://mui.com/material-ui/migration/migrating-from-jss/

-import { makeStyles } from '@material-ui/core/styles';
+import { makeStyles } from 'tss-react/mui';

Also it doesn't support functions for each individual style property. Only a single function is supported

I didn't use the following version, because it doesn't work:

import { makeStyles } from '@mui/styles';

I guess that now it's recommended to use styled instead of makeStyles. But it's your decision, so I left makeStyles as is with slight syntax changes.

When we migrated our own app to MUI 5 we decided to drop CSS-in-JS completely, because:

• Modern CSS is just fine, it supports everything required, and there is no need for an additional abstraction level
• You have to learn CSS-in-JS, but the amount of documentation is way lower in comparison to plain CSS
• Plain CSS is much easier to write, maintain and understand. It's impossible to use some complicated functions in CSS
• When you separate TypeScript code and CSS styles in different files it's easier to understand and maintain the app
• CSS is a standard, it's backward compatible. You don't have to rewrite all styles after each library update. Who knows, maybe authors of React and MUI are planing to change CSS-in-JS library after each major release and maybe ten years later they will back to plain CSS :)
• We use CSS-classes in Cypress-tests instead of data-testid, ARIA attributes or explicit CSS. For instance integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts at check .should('have.css', 'border-bottom', '1px solid rgb(51, 176, 195)') is failed on my machine, because the border width is 0.8px. So I prefer to check existence of a CSS-class instead

But it's just our private opinion on CSS-in-JS, not absolute truth

7. I had to increase specificity of some styles because they were ignored:

-select: {
-  display: 'flex',
-  alignItems: 'center',
-},
+select: {
+  '&': {
+    display: 'flex',
+    alignItems: 'center',
+  },
+},

For example in packages/forms/frontend/sirius-components-widget-reference/src/modals/CreateModal.tsx

8. nth-child(2) in makeStyles caused runtime warnings so I replaced it by a separate CSS-class in packages/core/frontend/sirius-components-core/src/workbench/RepresentationNavigation.tsx

9. I had to add the following line at the beginning of packages/core/frontend/sirius-components-core/src/index.ts to fix the error "styled_default is not a function":

import '@mui/material/styles/styled';

See for details vitejs/vite#12423 (comment)

10. <Select> role attribute is changed from button to combobox in MUI . So I updated Cypress tests

11. <Tooltip> adds aria-label instead of title to a nested <Link>

12. Tooltips works for disabled buttons only if the later one is surrounded by span

https://mui.com/material-ui/react-tooltip/#disabled-elements

<Tooltip title="Arrange all elements">
+ <span>
    <IconButton
      disabled={readOnly}>
      <AccountTreeIcon />
    </IconButton>
+ </span>
</Tooltip>

13. theme.spacing(1) returns string 8px instead of number 8. So I had to add parseInt() in packages/portals/frontend/sirius-components-portals/src/representations/PortalRepresentation.tsx

14. theme.palette.text.hint is removed theme.palette.text.hint is going away mui/material-ui-pickers#1681

15. Slight MUI 5 API changes:

-<Tabs scrollButtons="on" />
+<Tabs scrollButtons />

-<MenuItem button />
+<MenuItem />

16. It seems that <Popper> doesn't support transition attribute. If the attribute is specified then Popper is shown in the left upper corner.

Removed trnasition from packages/diagrams/frontend/sirius-components-diagrams/src/renderer/palette/tool-section/ToolSection.tsx and packages/diagrams/frontend/sirius-components-diagrams/src/renderer/palette/group-tool/GroupPalette.tsx

Cypress tests

I've fixed most of the tests after migration. But there are still some problems. Tests work unstable on my machine. Maybe it works better in your environment. Could you please help to fix it?

Some tests are failed when Cypress is run in a headless mode, probably because of problems with a clipboard or something else.

When run in an interactive mode the following tests are failed for Chrome:

1. integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts

expected '<div.nopan>' to have CSS property 'border-bottom' with the value '1px solid rgb(51, 176, 195)', but the value was '0.8px solid rgb(51, 176, 195)'

I tried to change Zoom factor in a browser, but it doesn't help. I think the test should not rely on a particular border width.

For Electron the following tests are failed:

1. integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts

In file integration-tests/cypress/workbench/Explorer.ts the test is failed on cy.getByTestId('new-object-modal').should('not.exist'). But if I add cy.wait(2000) before this line, the test is passed:

  public createObject(objectTreeItemLabel: string, childCreationDescriptionLabel: string) {
    this.getTreeItemByLabel(objectTreeItemLabel).find('button').click();
    cy.getByTestId('new-object').click();

    cy.getByTestId('childCreationDescription').children('[role="combobox"]').invoke('text').should('have.length.gt', 1);
    cy.getByTestId('childCreationDescription').click();
    cy.getByTestId('childCreationDescription')
      .get(`[data-value="${childCreationDescriptionLabel}"]`)
      .should('exist')
      .click();
    cy.getByTestId('create-object').click();
    // The test is fixed if I add the following line:
    // cy.wait(2000);
    cy.getByTestId('new-object-modal').should('not.exist');
  }

2. integration-tests/cypress/e2e/project/diagrams/group-palette.cy.ts

Error: "expected [data-testid="GroupPalette"] to exist in the DOM"

3. integration-tests/cypress/e2e/project/diagrams/node-aspect-ratio.cy.ts

Error: "Expected to find element: [data-testid="option-Node"], but never found it."

Vitest

React component snapshots are outdated after migration. And to be honest I have no idea how to fix it

[AxelRICHARD]

Hello,

Thank you very much for this detailed ticket and the associated pull request!
We will take a look at it, in the next weeks.

To update the snapshots, you first have to delete them, an then execute the test script in the dedicated package.
In your case, it seems you have to execute node run test in packages/core/frontend/sirius-components-core/.

Last thing, you commit messages should look like this:

[3437] Title

Bug: https://github.com/eclipse-sirius/sirius-web/issues/3437
Signed-off-by: NAME <EMAIL>

(you can also check commits in git history to see the format of the commit messages)

Regards,

[AresEkb]

Thanks for the answer! I've added Bug and Signed-off-by to commits. And fixed errors for sirius-components-core.

But now there are a lot off errors for sirius-components-forms, because format of CSS class names is changed:

<div
-  class="makeStyles-subscribers-126"
+  class="css-1aqjzxi-subscribers"
/>

(Just a side note: That's one more reason to dislike CSS-in-JS :) From my point of view these autogenerated CSS class names only add complexity and improve nothing)

New makeStyle() function from TSS React is based on @emotion/react. Here is a plugin @emotion/jest that should fix class names: https://emotion.sh/docs/testing But it's for jest only. There is no such a library for vitest yet: emotion-js/emotion#3132

I'll try to find a way to remove random hashes from CSS class names...

[AresEkb]

I've fixed vitest snapshots

Defined a custom snapshot serializer in packages/forms/frontend/sirius-components-forms/vitestSerializer.ts that transforms CSS class names from css-hash-suffix to makeStyles-suffix

Updated vitest to latest version, because snapshotSerializers option was added to vite.config.js recently: vitest-dev/vitest#2023

Also I've fixed some Cypress tests. It seems that .click('top') doesn't work well in Electron. I replaced it by .click(1, 1) in integration-tests/cypress/e2e/project/diagrams/collapse.cy.ts. And added the following code to this test:

cy.on('uncaught:exception', (err) =>
  err.message.includes('ResizeObserver loop limit exceeded') ? false : undefined
);

For sure it would be better to find an origin of the error and fix it. But it's hard to reproduce, it seems to be a known bug and people recommend to ignore it https://stackoverflow.com/a/50387233/632199

Two Cypress tests are still unstable:

• integration-tests/cypress/e2e/project/forms/widget-reference.cy.ts
• integration-tests/cypress/e2e/project/portals/portals.cy.ts

They work fine locally, so I can't reproduce the problem. However in GutHub they sometimes pass, sometimes fail.