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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Yarn 4 #21345

Draft
wants to merge 50 commits into
base: trunk
Choose a base branch
from
Draft

Upgrade to Yarn 4 #21345

wants to merge 50 commits into from

Conversation

igorschoester
Copy link
Member

@igorschoester igorschoester commented Apr 28, 2024

Context

Summary

This PR can be summarized in the following changelog entry:

  • Upgrades Yarn to v4.
  • [@yoast/ui-library 0.1.0] Renames the clean:build script to clean.
  • [@yoast/jest-preset 0.1.0 enhancement] Removes Yoast and lodash-es exceptions from the transformIgnorePatterns.
  • [@yoast/jest-preset 1.0.0 other] Upgrade jest peer dependency from version ^27.5.1 to ^29.7.0.
  • [@yoast/jest-preset 1.0.0 other] Add jest-environment-jsdom with version ^29.7.0 as peer dependency.

Relevant technical choices:

Based the branch on the yoast-components removal branch (#21344). As that probably helps skipping some old dependencies.

Add Yarn 4

  • Manual removal of .yarnrc and .yarn to be able to set to use Yarn 4
  • Running yarn set version berry, which made the above's Yarn 4 variants
  • Package json files are auto-adjusted by Yarn 4, except for the nohoist removal which is no longer supported (and unused anyway -- we don't have that dependency)
  • Lock file was auto-updated on install
  • Updated VERSIONS.json to mention the new file .yarnrc.yml
  • Added Yarn specific to the git ignore file as per https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored

The commits that follow are based on output from yarn install or what I came across trying the scripts in each package

  • Add missing dependencies
  • Refactor lifecycle scripts (pre- and post-)
  • Packages need their own tooling now it won't just magically work
  • Upgraded packages of which that seemed relatively straight forward to do so (if complaining for older React version or something)

Rename UI library clean:build to clean as our other packages have that instead

Ran into problems with the Indexation tests, seems it won't behave nicely with React 18 still is my best guess. So I refactored it: c5320d8

  • an error would result in an infinite loop: add url = false
  • an error would not result in the error being displayed: add wait for next cycle

Might be some scope creep while I was there:

  • Clean Jest config
  • Specify lerna/nx task dependencies
    • This one in particular makes Lerna pick up and build packages when needed, as long as you run the commands via Lerna (e.g. using yarn test --scope @yoast/wordpress-seo instead of cd packages/js && yarn test)

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Test all the things 馃槄

  • All the scripts
  • All the grunt tasks
  • Smoke test everything

But here are some specific areas:

  • The indexation: both in Tools and the First time configuration
  • a11y speak in our Settings
    • Test in Safari
    • Changing the route (e.g. from Site features to Site basics) should announce the name (e.g. Site basics Settings - Yoast SEO)
    • You can also verify the content of #a11y-speak-polite changes to it (inner workings of WP a11y)
    • Searching should announce how many results are found
  • Clicking outside of our (older) modals
    • In both Elementor and the block editor, close the following modals by clicking outside of them (was originally disabled due to warnings/errors)
    • SearchAppearanceModal
    • SocialAppearanceModal
    • SEMrushRelatedKeyphrasesModal
    • WincherSEOPerformanceModal (only changed in Elementor)
    • InsightsModal (only changed in Elementor)

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests to verify the code works as intended.
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes #

@igorschoester igorschoester added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Apr 28, 2024
Copy link

@igorschoester Please be aware that following packages have been abandoned and are not actively maintained anymore:

Package name Path
@yoast/babel-preset packages/babel-preset
@yoast/components packages/components
@yoast/e2e-tests packages/e2e-tests
@yoast/helpers packages/helpers
@yoast/jest-preset packages/jest-preset
@yoast/style-guide packages/style-guide

Please consider using the other packages instead.

* remove side-effect of publish, as per the recommendation in https://yarnpkg.com/advanced/lifecycle-scripts
* using files determine what we would publish, see https://docs.npmjs.com/cli/v10/configuring-npm/package-json#files
* ignore .map files, just in case the build contains them
* move browserslist-config index out of `src` folder, just for convenience of no `src` in the published version
Tweak lint to use .eslintignore file instead of only running in the src and tests folders
Move lint ignore to separate file
* upgrade @wordpress/components to WP 6.4 version, fixes a test failing on UUID sub dependency having require
* move lint ignore to separate file
* add own config
* auto-fix comma-dangle
* add own config
* auto-fix comma-dangle
* rename clean:build to clean, inline with the other packages
* prefix node commands with yarn, as per their recommendedation when using PnP linking mode: https://yarnpkg.com/migration/pnp#enabling-yarn-pnp
* switch to lint all vs just src (ignore missing test deps)
* add missing jest jsdom, even though tests are disabled
* introducing ignore file
* tune the max warnings to the current number
* move to JS format
* remove exceptions
* change react to detect
* add css/dist/ to ignore list
* auto-fix comma-dangle
* starting with the upgrade in our jest-preset
* upgrade the other packages to match
* fixes needed, coming in next commits
* an error would result in an infinite loop: add url = false
* an error would not result in the error being displayed: add wait for next cycle
* the above fixes the indexation tests
* missing act
* the closing would result in further state changes that were not captured by the fireEvent.click
* solution: wrap next check in waitFor
* remove the UI library source from the module mapping, it is expected to run build before test instead
* remove different node_module overrides
* add WP i18n to the module mapping to ensure a singleton, this fixes the text-domain warnings
* see https://lerna.js.org/docs/concepts/task-pipeline-configuration
* upgrade lerna
* ran lerna repair
* specify ignore cache in grunt tasks, just to ensure no change there
* add clean task (handy for cache busting too)
* alphabetize scripts
* skipping preset upgrades - lets not require minor upgrades
* feature-flag: remove unused babel plugins
* align versions
* fix missing tailwindcss in root
* fix wrong tailwindcss/forms version (not as per postcss-preset requirement)
* remove unused from JS
* add missing webpack peer dep to UI lib
* move to actual deps: @yoast/helpers and styled-components
* remove unused json dep
* add peer deps: react and react-dom
* add browserslist config to extend the yoast config
* @reduxjs/toolkit requires react-redux 7 or 8
* react-redux 8 requires redux 4
* remove redux-thunk and unused script that was using it
* add react and react-dom to root, to satisfy peer deps
* remove unused @testing-library/react-hooks, warned about wanting older version of react-renderer (16.9.0 or 17.0.0 vs actual 18.2.0)
* as peer deps of the packages using replacement-variable-editor
* required by grunt-contrib-clean, grunt-shell and grunt-git
* update grunt-git to work with grunt 1.6.1
* because it supports React 18
* turns out the close on click outside error is fixed too:
* removing shouldCloseOnClickOutside false for: SearchAppearanceModal, SocialAppearanceModal, SEMrushRelatedKeyphrasesModal, WincherSEOPerformanceModal (Elementor) and InsightsModal (Elementor)
* not using React 18 (but 16 at the latest)
* inactive package since 5 years
* still requiring React 16.3.x
* moving over to already used WP a11y' speak
* adding a hook and wrapper component for ease of use, as per downside of original PR: #19615
* due to warning about React 18 (package was using 17 before)
* was requiring old React version (via react-test-renderer)
* not really used at the moment
(or packages provided by WP)
* trigger was some requiring older React version -- not totally solved yet:
  * WP components at WP 6.5 version will solve Reakit
  * WP block-editor still requires react-autosize-textarea, which is archived since March 1, 2024
  * WP compose and WP data still use use-memo-one 1.1.2 or 1.1.1 where 1.1.3 has React 18 support
* we are not providing these packages in our plugin, so this is only affecting testing and development
* no more transform ignore or mapping of Yoast packages or lodash-es
* remove unused find-with-regex
* fix test import of lodash-es to use lodash
* they are required as peer deps by @yoast/style-guide now
* and it makes a bit more sense that it is actually used, even though it is mostly indirectly via WP element
* using versions from @yoast/grunt-plugin-tasks range to prevent potential problems
* fixing version of @yoast/grunt-plugin-tasks
There are no tests in the root folder
@coveralls
Copy link

Pull Request Test Coverage Report for Build 5a20fcf4642b15b29d1556e134a05a06ad13bdbe

Details

  • 12 of 20 (60.0%) changed or added relevant lines in 8 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.05%) to 52.413%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/helpers/src/ajaxHelper.js 0 1 0.0%
packages/js/src/settings/hooks/use-document-title.js 0 1 0.0%
packages/social-metadata-previews/src/editor/SocialPreviewEditor.js 0 1 0.0%
packages/js/src/first-time-configuration/tailwind-components/steps/indexation/indexation.js 0 2 0.0%
packages/js/src/settings/components/route-layout.js 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
packages/js/src/containers/SEMrushRelatedKeyphrasesModal.js 1 0.0%
packages/js/src/settings/components/route-layout.js 2 0.0%
Totals Coverage Status
Change from base Build ebae401160dbb57f6d2a8da5a0b29c42cda002dd: 0.05%
Covered Lines: 28476
Relevant Lines: 54959

馃挍 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants