neve
neve copied to clipboard
Codeinwp
[DEV] Update NPM packages and JS build
Summary
- Updated NPM packages to their latest version đ
- Using
wp-scriptsbuilt-in build for JS files instead of a custom one withts-loader. - Update the Rollup config files. File renamed to
.mjsandsourceMaptosourcemap. - The build can be run with Node 16, but Node 18 is the recommended way.
- Remove unused libs like
ts-loader,css-loader, etc.
âšī¸ The newer version of Eslint is more strict, so we have new warnings like:
/Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/assets/apps/customizer-controls/src/rich-text/RichTextComponent.js
49:5 warning React Hook useEffect has missing dependencies: 'isVisible' and 'section'. Either include them or remove the dependency array react-hooks/exhaustive-deps
58:5 warning React Hook useEffect has missing dependencies: 'controlId', 'isVisible', and 'updateValues'. Either include them or remove the dependency array react-hooks/exhaustive-deps
/Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/assets/apps/customizer-controls/src/spacing/SpacingComponent.js
102:5 warning React Hook useEffect has missing dependencies: 'control.id', 'defaultValue', 'dependsOn', and 'updateControlValue'. Either include them or remove the dependency array react-hooks/exhaustive-deps
/Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/assets/apps/customizer-controls/src/toggle/ToggleComponent.js
17:5 warning React Hook useEffect has missing dependencies: 'control.id', 'toggleValue', and 'value'. Either include them or remove the dependency array react-hooks/exhaustive-deps
Build Optimizations
When new versions change, this change has slightly increased the build size, even though some things are much smaller.
For CSS, the last version of the SASS compiler can have some smart replacements like :nth-child(even) to :nth-child(2n), which saves some space. It is stuck at some basic things like removing the space in this structure: var(--a, var(--b)), which adds up and thus makes it cross the limit.
Fortunately, with a new version comes new opportunities. With CSS minifier, we can use level 2 optimization to make some smart arrangements in the selector to reuse more code. This option enabled the size to be reduced to 1kb đ
It is a big win for CSS, but not for JS...
For the JS part, it was a bit sad of a change; the new Babel version has new tricks to deal with JS, and those new features increased the size of the scripts:
Old Babel Version
proposal-numeric-separator { "ios":"12.2" }
proposal-logical-assignment-operators { "firefox":"78", "ios":"12.2", "opera":"88", "samsung":"17" }
proposal-nullish-coalescing-operator { "ios":"12.2" }
proposal-optional-chaining { "ios":"12.2" }
syntax-json-strings { "android":"103", "chrome":"102", "edge":"103", "firefox":"78", "ios":"12.2", "opera":"88", "safari":"15.5", "samsung":"17" }
syntax-optional-catch-binding { "android":"103", "chrome":"102", "edge":"103", "firefox":"78", "ios":"12.2", "opera":"88", "safari":"15.5", "samsung":"17" }
syntax-async-generators { "android":"103", "chrome":"102", "edge":"103", "firefox":"78", "ios":"12.2", "opera":"88", "safari":"15.5", "samsung":"17" }
syntax-object-rest-spread { "android":"103", "chrome":"102", "edge":"103", "firefox":"78", "ios":"12.2", "opera":"88", "safari":"15.5", "samsung":"17" }
transform-template-literals { "ios":"12.2" }
proposal-export-namespace-from { "firefox":"78", "ios":"12.2", "safari":"15.5" }
syntax-dynamic-import { "android":"103", "chrome":"102", "edge":"103", "firefox":"78", "ios":"12.2", "opera":"88", "safari":"15.5", "samsung":"17" }
New Babel Version:
transform-unicode-sets-regex { chrome < 112, firefox < 116, ios, opera_mobile < 75, safari < tp, samsung }
proposal-class-static-block { ios < 16.4, safari < 16.4 }
syntax-private-property-in-object
syntax-class-properties
syntax-numeric-separator
syntax-nullish-coalescing-operator
syntax-optional-chaining
syntax-json-strings
syntax-optional-catch-binding
syntax-async-generators
syntax-object-rest-spread
syntax-export-namespace-from
syntax-dynamic-import
syntax-top-level-await
syntax-import-meta
As you can see, a lot of functionality in the old Babel version was at the proposal stage. And because of this, the new code slightly differs from the old one.
In the Rollup configuration file, you will see that I disabled: '@babel/plugin-transform-parameters' which does this -- the change was not present in the Old version.
For frontend.js, the limit was above with under 100B, so I put a little trick like making an alias for document to reduce the size. It worked đ§ . But for shop.js, the trick was not possible since most of the code is from a third-party lib, and the new Babel version introduced some additional vars and increased the size with 234 B -- for this, I think we need to increase the limit đĸ
â ī¸ Yarn changed with NPM
The new version of Eslint is not working with Yarn. It gives an error like:
yarn run lint
yarn run v1.22.21
$ npm-run-all lint:*
$ wp-scripts lint-js ./assets/js/src/
There was a problem loading formatter: /Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/node_modules/eslint/lib/cli-engine/formatters/stylish
Error: require() of ES Module /Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/node_modules/strip-ansi/index.js from /Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/node_modules/eslint/lib/cli-engine/formatters/stylish.js not supported.
Instead change the require of index.js in /Users/robert/Desktop/sites/dev/app/public/wp-content/themes/neve/node_modules/eslint/lib/cli-engine/formatters/stylish.js to a dynamic import() which is available in all CommonJS modules.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: "lint:global" exited with 2.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
So I had to change to NPM, which gives proper package support.
Will affect visual aspect of the product
NO
Screenshots
Test instructions
Since it is a big update on packages, there is a possibility for broken packages. Also, we need to keep in mind the compatibility.
- Make two instances, one with the latest version of WP and another with WP5.5 (our minimum). Also, install Neve Pro.
- Load the version from the PR and check for errors.
â ī¸ When encountering an error, please post a screenshot an share the instance. We need to double-check to see if it is a new error introduced in this or if it also happened in the current version of Neve, and we just found out about it.
Check before Pull Request is ready:
- [ ] I have written a test and included it in this PR
- [x] I have run all tests and they pass
- [x] The code passes when running the PHP CodeSniffer
- [X] Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
- [X] Security and Sanitization requirements have been followed
- [x] I have assigned a reviewer or two to review this PR (if you're not sure who to assign, we can do this step for you)
Closes #4143
The Gutenberg toolchain just dropped support for Node < 18 in the newest version. https://github.com/WordPress/gutenberg/blob/cdd6cecbc5bbf5ce5f55da0301246acfcf56c426/packages/scripts/CHANGELOG.md#2700-2024-01-10
The packages in this PR are all compatible with Node 18. Until this PR gets merged into the main, we will keep compatibility with Node 16 because of the Translations Diff job, which compares with the main (in this PR case, the old packages)
@preda-bogdan, the PR is done. I waiting for the update on E2E regarding visual regression. Everything should be green before we start testing.
@Soare-Robert-Daniel So the status is that is awaiting the automatic checks to pass before being submitted to Code Review and testing. My question was related to if this should be included in this next release or not. I assume we will skip this for now until everything is complete.
@Soare-Robert-Daniel here, I don't understand how having a bigger build overall is better. Also, I am not confident that this will not break the CI jobs once it reaches development and master. How can we ensure that the changes introduced are safe? There are a lot of changes all introduced at once.
@GrigoreMihai What is your opinion on this?
@preda-bogdan The build is a little over the limit because of the updated Babel version. In the old version, some JS features were still in proposal mode, which means that there were some changes when I switched to stable.
@preda-bogdan I agree with you on the fact that there are a lot of changes at once and will be hard to track down a bug if it reaches production.
What I'm thinking is if we focus on adding more automated tests before this update so we will reach a point where we are more confident that changes don't introduce bugs? But this might take a while.
@Soare-Robert-Daniel What I'm thinking if it will be possible to split this into multiple parts ? And let's say we leave the babel update at the very end since it is adding a bit of size.
Also for babel would it be possible to check it's config options https://babeljs.io/docs/options to see if there are features we can disable during bundling (like the ones that we don't need from the new added ones) ? My idea would still be that an package update should not increase the bundle size if we keep the same config, of course if you think it's possible.
The waryness about the build size being a few bytes more is not translated to possible error.
The new build has just another arrangement.
Example:
Here, you can see the new version moves the var outside the for loop and no longer creates the o var, which takes the value of t.lenght.
Why does it do that? It does because that var additional var is not needed. It is an unused var; you can check the code here: https://github.com/ganlanyuan/tiny-slider/blob/4d709735c417c2483e77a22d017fc1b18c04f0d4/src/helpers/whichProperty.js#L16-L21
As you can see, the new version structure is more similar to the original code.
But this similarity is not cheap since we now have two declarations of var keyword with it its needed space between keyword and var
Another example is this one:
As you can see, it is just a different arrangement.
Same thing here:
The new build uses a var to store the string. Like in the source code https://github.com/ganlanyuan/tiny-slider/blob/4d709735c417c2483e77a22d017fc1b18c04f0d4/src/tiny-slider.module.js#L375C27-L375C43
The old build just inlined the string where it was used:
If you want to see more details check the zip file with the build file from the master vs the one in this PR. All of them were formatted to have a nice visualization with diff
Also if you check other build files, you can find intresting things like this:
!l.toString.toString().includes("[native code]") is a check to make sure that the base function was not tempered.
Now, the shop.js is below the limit. No need for a new limit đĨ
Since all the shop.js dependencies do not have classes or abuse of this, we can be more aggressive with the use of the function arrow:
The save is not great, but not terrible:
assets/js/build/modern/frontend.js
Size limit: 7 kB
Size: 6.93 kB with all dependencies and minified
Loading time: 136 ms on slow 3G
assets/js/build/modern/shop.js
Size limit: 33 kB
Size: 32.67 kB with all dependencies and minified
Loading time: 639 ms on slow 3G
style-main-new.min.css
Size limit: 38.1 kB
Size: 36.99 kB with all dependencies and minified
Loading time: 723 ms on slow 3G
@Soare-Robert-Daniel Did some testing and found a single issue for now:
- [ ] After activating all items in the post meta for Blog/archive, the meta wasn't visible anymore neither on the blog page or in a single post . That happened using only Neve and importing the Pet shop starter site. When I tried to reorder the post meta, they started to work again.
- [ ] Something similar happened when I added another icon in the Social icons header component. They were visible again after reordering https://vertis.d.pr/v/KkoMMt
- [ ] ^ The same happened for the Payment icons in the footer
Testing is not complete so I'll add other comments if I find anything else
- [ ] Warnings and deprecated errors using WordPress 5.5, also the content of the posts/pages is not loaded https://vertis.d.pr/v/KNEE8Y
- [ ] Fatal error related to Neve using WordPress 5.6 - 5.8, on all pages https://vertis.d.pr/i/v41SPu
Can be checked on this instance:
https://packageupdate.s1-tastewp.com/wp-admin/
user: test
password: test
- [ ] After activating all items in the post meta for Blog/archive, the meta wasn't visible anymore neither on the blog page or in a single post . That happened using only Neve and importing the Pet shop starter site. When I tried to reorder the post meta, they started to work again.
- [ ] Something similar happened when I added another icon in the Social icons header component. They were visible again after reordering https://vertis.d.pr/v/KkoMMt
- [ ] ^ The same happened for the Payment icons in the footer
Those are now fixed đĨ
The version in npm package for react-sortablejs was set to ^6.0.0, and when I did the update, the lock file was saved, a newer version ( the yarn lock had the 6.0.0).
What was the problem? The component used for sorting the icons augments the given list (in our case, the var list with additional keys (selected, chosen)), and we also use list to send the updates to the db.
Until version 6.0.4, the component made a shallow copy of each item along with the augmentation. The functions that were working directly with sorting functions were aware of this behavior and cleaned up the keys because otherwise, the server sanitization would reject it.(that's why sorting was working as expected)
On version 6.0.4, the devs changed the behavior so that the sorting component will preserve the original list. Thus, the augmented keys were added to the initial list, poisoning all the functions (like the Add Item button).
To solve this, I pinned down to version to 6.0.3 to always install this version regarding how you run it with npm or yarn.
Ideally, we should use a separate list for sorting and another one for storing the data that is going to the database to make sure the library is not making unwanted changes.
- [ ] Warnings and deprecated errors using WordPress 5.5, also the content of the posts/pages is not loaded https://vertis.d.pr/v/KNEE8Y
- [ ] Fatal error related to Neve using WordPress 5.6 - 5.8, on all pages https://vertis.d.pr/i/v41SPu
This happens also in the current version. Made an issue about it https://github.com/Codeinwp/neve-pro-addon/issues/2804