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

Bundle optimization #5295

Merged
merged 47 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
de9dcbd
Added @loadable/component typings
pnicolli Oct 7, 2023
661c130
wip: lazyload Form components and Diff view, still need to fix most t…
deodorhunter Oct 7, 2023
acb3477
Code split Contents, RelationsCP, UsersCP, RulesCP
pnicolli Oct 7, 2023
e4433cb
wip: common webpack bundle name for Form related stuff
deodorhunter Oct 7, 2023
51000e4
wip: merge with base bundle optimizations
deodorhunter Oct 7, 2023
f925d01
Merge branch 'main' into bundle-optimization
pnicolli Oct 7, 2023
34e6530
Merge branch 'main' into bundle-optimization
pnicolli Oct 7, 2023
0b8a907
Merge branch 'main' into bundle-optimization
mamico Oct 13, 2023
734dbcc
Code split all controlpanels, form components and widgets
pnicolli Nov 7, 2023
35dd3cc
Merge branch 'main' into bundle-optimization
pnicolli Nov 7, 2023
17b803e
added babel-plugin-transform-imports
pnicolli Nov 17, 2023
48bbe90
Merge branch 'main' into bundle-optimization
pnicolli Nov 29, 2023
2a3b8b7
Removed babel-plugin-transform-imports and readded imports in compone…
pnicolli Dec 16, 2023
b1f39f7
fix chunk name clash
pnicolli Dec 19, 2023
779146f
Merge branch 'main' into bundle-optimization
pnicolli Dec 23, 2023
5a0c0fc
reinstalled @types/loadable__component
pnicolli Dec 23, 2023
5bd41bf
Merge branch 'main' into bundle-optimization
pnicolli Jan 6, 2024
680d147
Merge branch 'main' into bundle-optimization
mamico Jan 11, 2024
ae658ed
Merge branch 'main' into bundle-optimization
mamico Jan 11, 2024
8717f1e
fix(test): edit comment
mamico Jan 15, 2024
0b12d9e
Merge branch 'main' into bundle-optimization
pnicolli Jan 16, 2024
799348e
Cleanup and news item
pnicolli Jan 16, 2024
ce86356
Added volto-slate news item
pnicolli Jan 16, 2024
d221aad
fix: remove loadable where component is composed
mamico Jan 16, 2024
f48fb47
Revert "fix: remove loadable where component is composed"
pnicolli Jan 29, 2024
0bc13f2
Removed useless compose
pnicolli Jan 29, 2024
2e399a0
Fix some cypress tests
pnicolli Jan 29, 2024
fb08a8f
Merge branch 'main' into bundle-optimization
pnicolli Jan 29, 2024
f4fb67e
Merge branch 'main' into bundle-optimization
pnicolli Jan 30, 2024
d7540f7
Merge branch 'main' into bundle-optimization
pnicolli Apr 2, 2024
2121682
restore new deps
pnicolli Apr 2, 2024
8cd2b52
unit tests
pnicolli Apr 2, 2024
1350ead
more tests fixes
pnicolli Apr 2, 2024
8329b22
Merge branch 'main' into bundle-optimization
pnicolli Apr 8, 2024
89e1cc9
wip on controlpanels
pnicolli Apr 8, 2024
4d55d66
more test fixes, controlpanels view now works correctly
pnicolli Apr 9, 2024
1aaecc5
added docs
pnicolli Apr 9, 2024
f2e54ab
Merge branch 'main' into bundle-optimization
pnicolli Apr 16, 2024
2ec660b
fix slate cypress test
pnicolli Apr 16, 2024
23c0365
Merge branch 'main' into bundle-optimization
pnicolli Apr 17, 2024
713fd29
rename docs file
pnicolli Apr 17, 2024
cfbcac4
Add introduction and minor grammar, spelling, and style fixes
stevepiercy Apr 17, 2024
cc1714a
Revert editor changes
stevepiercy Apr 17, 2024
34cfadc
Merge branch 'main' into bundle-optimization
stevepiercy Apr 17, 2024
46af613
Merge branch 'main' into bundle-optimization
pnicolli Apr 18, 2024
5343b27
fix more slate tests
pnicolli Apr 18, 2024
9218817
Merge branch 'main' into bundle-optimization
sneridagh Apr 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions docs/source/contributing/bundle-size-optimization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
myst:
html_meta:
"description": "Bundle size optimization in Volto"
"property=og:description": "Bundle size optimization in Volto"
"property=og:title": "Bundle size optimization"
"keywords": "Volto, Plone, frontend, React, Performance, guidelines"
---

(bundle-size-optimization-label)=

# Bundle size optimization

This document describes how to optimize the bundle size of components through {term}`lazy loading` to improve page load times.
Contributors to Volto core should follow the guidelines in this document.


## Lazy loading in core

Since Volto 18, several core components use lazy loading to keep the final build size under control.

For example, the `Form` components are lazy loaded, which means that the code for the form components is only loaded when the user navigates to a page that contains a form.
A new index file has been created at `packages/volto/src/components/manage/Form/index.tsx` that exports the form components with lazy loading.
The `export`s in the main components index (`packages/volto/src/components/index.js`) have been updated to export components from the new specific index.
The same goes for other components that have been lazy loaded, such as the control panels and widgets.

Several `import` statements have been updated to use the new lazy loaded components.
For example, the `Form` component is now imported from `@plone/volto/components/manage/Form` instead of `@plone/volto/components/manage/Form/Form`.
You should keep this in mind, and always import components from the specific component index files when available, while avoiding importing components from the main components index file, if possible.
This should also help to reduce circular dependencies, and help the overall build performance in the long run.


### Unit test components that use lazy loaded components

If you import a component from a lazy loaded index, you can have issues with rendering these in unit tests.
Mocks are provided for lazy loaded components and are available for you to use.
This can be done by using the `jest.mock` function to mock the specific component index.
For example, to mock the `Form` component and all other components in the `Form`-specific index, you can use the following code in your test file:

```javascript
jest.mock('@plone/volto/components/manage/Form');
```
3 changes: 3 additions & 0 deletions docs/source/contributing/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ The Volto Team reviews pull requests only from people with a GitHub account who
```{include} ./branch-policy.md
```


(contributing-install-volto-for-development-label)=

## Install Volto for development

For developing Volto, follow {doc}`developing-core`.


(contributing-translations-label)=

## Translations
Expand Down Expand Up @@ -125,6 +127,7 @@ redux
routing
icons
accessibility-guidelines
bundle-size-optimization
typescript
volto-core-addons
version-policy
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useIntl } from 'react-intl';
import { BlockDataForm } from '@plone/volto/components';
import { BlockDataForm } from '@plone/volto/components/manage/Form';
import type { BlockEditProps } from '@plone/types';

const TestBlockData = (props: BlockEditProps) => {
Expand Down
1 change: 1 addition & 0 deletions packages/volto-slate/news/5295.internal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update imports to work with the new code split components in Volto. @pnicolli
3 changes: 2 additions & 1 deletion packages/volto-slate/src/blocks/Table/TableBlockEdit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import cx from 'classnames';
import { defineMessages, injectIntl } from 'react-intl';

import Cell from './Cell';
import { BlockDataForm, Icon, SidebarPortal } from '@plone/volto/components';
import { Icon, SidebarPortal } from '@plone/volto/components';
import { BlockDataForm } from '@plone/volto/components/manage/Form';
import TableSchema from './schema';

import rowBeforeSVG from '@plone/volto/icons/row-before.svg';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@ import {
validateFileUploadSize,
} from '@plone/volto/helpers';
import config from '@plone/volto/registry';
import {
BlockDataForm,
SidebarPortal,
BlockChooserButton,
} from '@plone/volto/components';
import { SidebarPortal, BlockChooserButton } from '@plone/volto/components';
import { BlockDataForm } from '@plone/volto/components/manage/Form';

import { SlateEditor } from '@plone/volto-slate/editor';
import { serializeNodesToText } from '@plone/volto-slate/editor/render';
Expand Down
3 changes: 2 additions & 1 deletion packages/volto-slate/src/elementEditor/PluginEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { isEqual } from 'lodash';
import React from 'react';
import { useDispatch } from 'react-redux';
import { ReactEditor } from 'slate-react';
import { Icon as VoltoIcon, BlockDataForm } from '@plone/volto/components';
import { Icon as VoltoIcon } from '@plone/volto/components';
import { BlockDataForm } from '@plone/volto/components/manage/Form';
import { setPluginOptions } from '@plone/volto-slate/actions';
import BaseSchemaProvider from './SchemaProvider';

Expand Down
2 changes: 1 addition & 1 deletion packages/volto-slate/src/widgets/HtmlSlateWidget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MemoryRouter } from 'react-router-dom';
import { Provider, useSelector } from 'react-redux';
import { defineMessages, injectIntl } from 'react-intl';

import { FormFieldWrapper } from '@plone/volto/components';
import { FormFieldWrapper } from '@plone/volto/components/manage/Widgets';
import SlateEditor from '@plone/volto-slate/editor/SlateEditor';
import { serializeNodes } from '@plone/volto-slate/editor/render';
import { makeEditor } from '@plone/volto-slate/utils';
Expand Down
3 changes: 2 additions & 1 deletion packages/volto-slate/src/widgets/ObjectByTypeWidget.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { Menu, Tab } from 'semantic-ui-react';
import { Icon, ObjectWidget } from '@plone/volto/components';
import { Icon } from '@plone/volto/components';
import { ObjectWidget } from '@plone/volto/components/manage/Widgets';

export const ObjectByTypeWidget = (props) => {
const { schemas, value = {}, onChange, errors = {}, id } = props;
Expand Down
2 changes: 1 addition & 1 deletion packages/volto-slate/src/widgets/RichTextWidget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import React from 'react';
import isUndefined from 'lodash/isUndefined';
import isString from 'lodash/isString';
import { FormFieldWrapper } from '@plone/volto/components';
import { FormFieldWrapper } from '@plone/volto/components/manage/Widgets';
import SlateEditor from '@plone/volto-slate/editor/SlateEditor';

import { createEmptyParagraph, createParagraph } from '../utils/blocks';
Expand Down
5 changes: 1 addition & 4 deletions packages/volto/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,7 @@ Cypress.Commands.add(
'pasteClipboard',
{ prevSubject: true },
(query, htmlContent) => {
return cy
.wrap(query)
.type(' {backspace}')
.trigger('paste', createHtmlPasteEvent(htmlContent));
return cy.wrap(query).trigger('paste', createHtmlPasteEvent(htmlContent));
},
);

Expand Down
9 changes: 9 additions & 0 deletions packages/volto/cypress/support/e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ import './commands';
import { setupGuillotina, tearDownGuillotina } from './guillotina';
import { setup, teardown } from './reset-fixture';

Cypress.on('uncaught:exception', (err) => {
// We are getting this error in Cypress tests but we don't use ResizeObserver ourselves
if (/ResizeObserver loop/.test(err.message)) {
// returning false here prevents Cypress from
// failing the test
return false;
}
});

before(function () {
if (Cypress.env('API') === 'guillotina') {
tearDownGuillotina({ allowFail: true });
Expand Down
3 changes: 2 additions & 1 deletion packages/volto/cypress/tests/core/controlpanels/upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ describe('Upgrade Site Tests', () => {
body: {
...getUpgradeNeedsUpgrade,
},
}).as('getSystemNeedsUpdate');
}).as('getUpgradeNeedsUpgrade');
Copy link
Member

Choose a reason for hiding this comment

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

Renamed, but then not used? watch out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sneridagh You have a point here. I changed it because the way I understand this file, there's a name clash. The original version of the test is:

  it('As manager, I can click upgrade site, the upgrade control panel shows', function () {
    cy.intercept('GET', '/**/@system', {
      statusCode: 200,
      body: {
        ...getsystemNeedsUpgrade,
      },
    }).as('getSystemNeedsUpdate');
    cy.intercept('GET', '/**/@upgrade', {
      statusCode: 200,
      body: {
        ...getUpgradeNeedsUpgrade,
      },
    }).as('getSystemNeedsUpdate');
    cy.navigate('controlpanel');
    cy.wait('@getSystemNeedsUpdate');

    cy.findByText('Please continue with the upgrade.').click();
    cy.get('.content-area').contains(
      'The site configuration is outdated and needs to be upgraded.',
    );
  });

If I understand correctly there are two different calls intercepted with the alias getSystemNeedsUpdate and I doesn't feel right. On the other hand, if I change it where do I wait for the second one? Maybe after the click?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried adding an additional wait after the click, I am not sure it is needed at all, but the request is actually made to the backend so I think it is worth waiting for it to actually happen, at least in the current state of things

Copy link
Member

Choose a reason for hiding this comment

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

I see, maybe we need to revisit this at some point.

cy.navigate('controlpanel');
cy.wait('@getSystemNeedsUpdate');

cy.findByText('Please continue with the upgrade.').click();
cy.wait('@getUpgradeNeedsUpgrade');
cy.get('.content-area').contains(
'The site configuration is outdated and needs to be upgraded.',
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ describe('Block Tests: Links', () => {
it('As editor I can add a link and pressing enter does not add another link in the next block', function () {
// https://github.com/plone/volto/pull/5186
cy.get('#toolbar').click();
cy.getSlate().type('Colorless green ideas sleep furiously');
cy.getSlateEditorAndType('Colorless green ideas sleep furiously');

cy.log('Create a Link');

Expand All @@ -111,8 +111,8 @@ describe('Block Tests: Links', () => {
'https://google.com{enter}',
);
cy.getSlate().should('have.descendants', 'a.slate-editor-link');
cy.getSlate().type('{rightarrow}').type('{enter}');
cy.getSlate().type('Hello').type('{enter}');
cy.getSlateEditorAndType('{rightarrow}').type('{enter}');
cy.getSlateEditorAndType('Hello').type('{enter}');

cy.toolbarSave();

Expand Down
1 change: 1 addition & 0 deletions packages/volto/news/5295.internal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduced JavaScript bundle size of the production build. Code split several internal modules: Controlpanels, Form, Widgets among other small ones. @pnicolli @deodorhunter
1 change: 1 addition & 0 deletions packages/volto/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@
"@testing-library/react": "14.2.0",
"@testing-library/react-hooks": "8.0.1",
"@types/jest": "^29.5.8",
"@types/loadable__component": "^5.13.9",
"@types/lodash": "^4.14.201",
"@types/react": "^18",
"@types/react-dom": "^18",
Expand Down
Loading
Loading